-
Notifications
You must be signed in to change notification settings - Fork 1.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
[Backport 1.x] Add regexp interval source #2069
Conversation
* Add regexp interval source Add a regexp interval source provider so people can use regular expressions inside of intervals queries. Signed-off-by: Matt Weber <matt@mattweber.org> * Fixes - register regexp interval in SearchModule - use fully-qualified name for lucene RegExp - get rid of unnecessary variable Signed-off-by: Matt Weber <matt@mattweber.org> (cherry picked from commit b9420d8)
Can one of the admins verify this patch? |
@mattweber are you saying this will need serialization checks on top of? bcw tests? |
@dblock yes this needs a few or the bwc tests will fail. With this merged, I can do PR for the proper checks. |
} | ||
|
||
public Regexp(StreamInput in) throws IOException { | ||
this.pattern = in.readString(); |
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.
Need version checks >= 1.3
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.
@mattweber do you think you need it? the 1.3.0+ would recognize case "regexp"
but older clusters would not, so we should not see the case when older cluster tries to reconstruct Regexp
from the stream
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.
Yea I am not sure, usually I do the checks for an existing feature not a new one. Maybe we just go as is and see if we start triggering any BWC failures. In which case I can do a followup PR for version checks.
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.
👍 It should be fine I think
|
||
@Override | ||
public void writeTo(StreamOutput out) throws IOException { | ||
out.writeString(pattern); |
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.
Version checks >= 1.3
Will also need to add the same checks to |
@reta can you please look at this? Now that I think of it I am not sure what the correct way to handle this is since it is a completely new feature and with a version check the entire |
@dblock I think we can merge this without any checks (see above comments). If any bwc checks start to fail I will open a follow-up PR to address it. Thank you! |
@dblock is this ok to be merged or would you like me to make some changes? |
start gradle check |
@mattweber For gradle check failures, check the log please, open/update existing issues for any new/known failures and I or any other admin can kick it again. |
start gradle check |
start gradle check |
... http://vault.centos.org/centos/ is having issues |
start gradle check |
start gradle check |
@dblock can you merge please? |
Backport b9420d8 from #1917