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

Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time #8534

Merged
merged 1 commit into from Nov 19, 2014

Conversation

Projects
None yet
3 participants
@martijnvg
Member

martijnvg commented Nov 18, 2014

PR for #8474. Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a now expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment instead of during parse time. In the case of the range filter a special Resolvable Filter makes sure that now is resolved when the DocIdSet is pulled and in the case of the range query now is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Note this PR is against the 1.x branch.

@s1monw

View changes

Show outdated Hide outdated src/main/java/org/elasticsearch/common/lucene/search/ResolvableFilter.java
@@ -363,6 +373,16 @@ public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Obje
* - the String to parse does not have already a timezone defined (ie. `2014-01-01T00:00:00+03:00`)
*/
public Filter rangeFilter(QueryParseContext parseContext, Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser, @Nullable QueryParseContext context, @Nullable Boolean explicitCaching) {
IndexNumericFieldData fieldData = parseContext != null ? (IndexNumericFieldData) parseContext.getForField(this) : null;

This comment has been minimized.

@s1monw

s1monw Nov 19, 2014

Contributor

why do we actually need to only wrap this in a LateParsingFilter if there is no context? can't we do this all the time and in our tests do the resolve instead? I don't like the special casing based on the context here to be honest

@s1monw

s1monw Nov 19, 2014

Contributor

why do we actually need to only wrap this in a LateParsingFilter if there is no context? can't we do this all the time and in our tests do the resolve instead? I don't like the special casing based on the context here to be honest

This comment has been minimized.

@martijnvg

martijnvg Nov 19, 2014

Member

I like to do that too, but this does mean that the resolve method is invoked for each segment that filter is going to execute on. See QueryParseContext#cacheFilter(...) around line 214.

Would this side effect be acceptable?

@martijnvg

martijnvg Nov 19, 2014

Member

I like to do that too, but this does mean that the resolve method is invoked for each segment that filter is going to execute on. See QueryParseContext#cacheFilter(...) around line 214.

Would this side effect be acceptable?

This comment has been minimized.

@s1monw

s1monw Nov 19, 2014

Contributor

I think we should just stick with what we have for now :/ filter needs a rewrite method

@s1monw

s1monw Nov 19, 2014

Contributor

I think we should just stick with what we have for now :/ filter needs a rewrite method

@s1monw

View changes

Show outdated Hide outdated ...va/org/elasticsearch/index/query/IndexQueryParserFilterCachingTests.java
@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Nov 19, 2014

Contributor

left some comments

Contributor

s1monw commented Nov 19, 2014

left some comments

@s1monw s1monw self-assigned this Nov 19, 2014

@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Nov 19, 2014

Member

@s1monw I applied the feedback and left one comment.

Member

martijnvg commented Nov 19, 2014

@s1monw I applied the feedback and left one comment.

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Nov 19, 2014

Contributor

LGTM

Contributor

s1monw commented Nov 19, 2014

LGTM

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534

@martijnvg martijnvg changed the title from Resolve `now` date ranges in percolator and alias filters at search time instead of parse time to Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time Nov 19, 2014

@martijnvg martijnvg merged commit bba6560 into elastic:1.x Nov 19, 2014

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Nov 19, 2014

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534

martijnvg added a commit that referenced this pull request Nov 19, 2014

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534

martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Nov 19, 2014

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534

@clintongormley clintongormley changed the title from Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time to Query DSL: Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time Nov 25, 2014

@clintongormley clintongormley added :Dates and removed review labels Mar 19, 2015

@martijnvg martijnvg deleted the martijnvg:bugs/alias_filter_and_percolator_query_with_now_based_range branch May 18, 2015

@clintongormley clintongormley changed the title from Query DSL: Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time to Resolve `now` in date ranges in percolator and alias filters at search time instead of parse time Jun 7, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

Core: Added query/filter wrapper that builds the actual query to be e…
…xecuted on the last possible moment to aid with index aliases and percolator queries using `now` date expression.

Percolator queries and index alias filters are parsed once and reused as long as they exist on a node. If they contain time based range filters with a `now` expression then the alias filters and percolator queries are going to be incorrect from the moment these are constructed (depending on the date rounding).

 If a range filter or range query is constructed as part of adding a percolator query or a index alias filter then these get wrapped in special query or filter wrappers that defer the resolution of now at last possible moment as apposed during parse time. In the case of the range filter a special Resolvable Filter makes sure that `now` is resolved when the DocIdSet is pulled and in the case of the range query `now` is resolved at query rewrite time. Both occur at the time the range filter or query is used as apposed when the query or filter is constructed during parse time.

Closes #8474
Closes #8534
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment