-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Query refactoring: SpanNearQueryBuilder and Parser #12156
Query refactoring: SpanNearQueryBuilder and Parser #12156
Conversation
public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuilder> implements SpanQueryBuilder<SpanNearQueryBuilder> { | ||
|
||
public static final String NAME = "span_near"; | ||
|
||
private ArrayList<SpanQueryBuilder> clauses = new ArrayList<>(); | ||
static boolean DEFAULT_IN_ORDER = true; | ||
static boolean DEFAULT_COLLECT_PAYLOADS = true; |
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.
shouldnt these constant be public?
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.
Can do. btw. collectPayloads
was lacking any documentation, also deep down in lucene land. Any idea where else to look, since when I make this public I'd like to have javadocs with it.
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.
you mean that our public getter for it has no javadocs either? :) you can have a look at NearSpansPayloadOrdered
in lucene but you won't find much more javadocs, we can simply state that the flag controls whether we collect payloads or not :)
looks good left a few minor comments |
@javanna left a few comments regarding your questions. |
7a19916
to
c9dbd7a
Compare
@javanna Added the comments and made default settings for flags public. |
/** Default for flag controlling whether matches are required to be in-order */ | ||
public static boolean DEFAULT_IN_ORDER = true; | ||
|
||
/** Default for flag controlling whether payload is collected */ |
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.
s/payload is/payloads are
LGTM besides the minor comment I left |
c9dbd7a
to
7efca09
Compare
Moving the query building functionality from the parser to the builders new toQuery() method analogous to other recent query refactorings. Relates to elastic#10217
7efca09
to
c689e89
Compare
…annear Query refactoring: SpanNearQueryBuilder and Parser
…ring-spannear Query refactoring: SpanNearQueryBuilder and Parser
This change is breaking for the java api as it removes setter for mandatory slop parameter which needs to be set in the constructor instead. |
Moving the query building functionality from the parser to the builders
new toQuery() method analogous to other recent query refactorings.
Relates to #10217
This PR is agains the query refactoring branch.