-
Notifications
You must be signed in to change notification settings - Fork 733
Add rewrite param setters to all relevant queries #2242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 9.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ | |
|
|
||
| namespace Elastica\Query; | ||
|
|
||
| use Elastica\Exception\InvalidException; | ||
|
|
||
| /** | ||
| * Regexp query. | ||
| * | ||
|
|
@@ -13,6 +15,21 @@ | |
| */ | ||
| class Regexp extends AbstractQuery | ||
| { | ||
| /** | ||
| * Rewrite methods: @see https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-multi-term-rewrite.html. | ||
| */ | ||
| public const REWRITE_CONSTANT_SCORE = 'constant_score'; | ||
| public const REWRITE_CONSTANT_SCORE_BOOLEAN = 'constant_score_boolean'; | ||
| public const REWRITE_SCORING_BOOLEAN = 'scoring_boolean'; | ||
| public const REWRITE_TOP_TERMS_BLENDED_FREQS_N = 'top_terms_blended_freqs_N'; | ||
| public const REWRITE_TOP_TERMS_BOOST_N = 'top_terms_boost_N'; | ||
| public const REWRITE_TOP_TERMS_N = 'top_terms_N'; | ||
|
|
||
| /** | ||
| * @var string|null | ||
| */ | ||
| private $field; | ||
|
|
||
| /** | ||
| * Construct regexp query. | ||
| * | ||
|
|
@@ -32,8 +49,28 @@ public function __construct(string $key = '', ?string $value = null, float $boos | |
| * | ||
| * @return $this | ||
| */ | ||
| public function setValue(string $key, ?string $value = null, float $boost = 1.0) | ||
| public function setValue(string $key, ?string $value = null, float $boost = 1.0): self | ||
| { | ||
| $this->field = $key; | ||
|
|
||
| return $this->setParam($key, ['value' => $value, 'boost' => $boost]); | ||
| } | ||
|
|
||
| /** | ||
| * Set the method used to rewrite the query. | ||
| * Use one of the Regexp::REWRITE_* constants, or provide your own. | ||
| * | ||
| * @see https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-multi-term-rewrite.html | ||
| */ | ||
| public function setRewrite(string $rewriteMode): self | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These methods are always identical?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the same for Prefix and Regexp, also for Wildcard except the null check on IMHO, these queries could use a refactor in the long run, to harmonize them and have consistent constructors. |
||
| { | ||
| if (null === $this->field) { | ||
| throw new InvalidException('No field has been set'); | ||
| } | ||
|
|
||
| $data = $this->getParam($this->field); | ||
| $this->setParam($this->field, \array_merge($data, ['rewrite' => $rewriteMode])); | ||
|
|
||
| return $this; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,9 @@ class Wildcard extends AbstractQuery | |
| public const REWRITE_CONSTANT_SCORE = 'constant_score'; | ||
| public const REWRITE_CONSTANT_SCORE_BOOLEAN = 'constant_score_boolean'; | ||
| public const REWRITE_SCORING_BOOLEAN = 'scoring_boolean'; | ||
| public const REWRITE_TOP_TERMS_BLENDED_FREQS_N = 'top_terms_blended_freqs_N'; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting that we had some of the const already |
||
| public const REWRITE_TOP_TERMS_BOOST_N = 'top_terms_boost_N'; | ||
| public const REWRITE_TOP_TERMS_N = 'top_terms_N'; | ||
|
|
||
| /** | ||
| * @var string | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these const always the same. I wonder if we should define these once in AbstractQuery. But then, they don't apply to all queries. What is the pattern of all the queries this applies to? Should we have an abstract query or similar that all these extend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These constants are always the same. The only pattern between these, is that they are all Term-level queries that use wildcards in some shape or form.
When writing the PR I was also debating puttings the constants in AbstractQuery but reached the same conclusion.
I guess we could have a new level of abstract query for these. Not sure how to name it though.