You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/05/06 07:24:47 UTC
[GitHub] [druid] suneet-s opened a new pull request #9830: Fail incorrectly constructed join queries
suneet-s opened a new pull request #9830:
URL: https://github.com/apache/druid/pull/9830
### Description
This change validates an assumption made HashJoinSegmentStorageAdapter. It was assumed that the filter used during join filter pre-analysis is the same as the filter passed to the HashJoinSegmentStorageAdapter to make a cursor. As pointed out in #9792, this is not always the case. This change makes it so that any query that violates this assumption fails rather than showing incorrect results.
The issue is surfaced when nested groupby queries operate on a non-table datasource. This is because the GroupByQueryQueryToolChest has optimizations built in to it that pushes down the filters from the outer group by to the inner query when making the cursor. So the pre-join analysis is invalid. The result of this bug is that as of 0.18 filters in the outer groupBy are ignored.
This PR fails the query rather than providing the user with invalid results. A follow up PR is needed to address #9792 so that the query executes correctly.
This PR has:
- [x] been self-reviewed.
- [x] added unit tests or modified existing tests to cover new code paths.
- [x] added integration tests.
- [x] been tested in a test Druid cluster.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jon-wei commented on a change in pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
jon-wei commented on a change in pull request #9830:
URL: https://github.com/apache/druid/pull/9830#discussion_r424673216
##########
File path: processing/src/main/java/org/apache/druid/segment/filter/RegexFilter.java
##########
@@ -100,7 +100,7 @@ public boolean equals(Object o)
return false;
}
PatternDruidPredicateFactory that = (PatternDruidPredicateFactory) o;
- return Objects.equals(pattern, that.pattern);
+ return Objects.equals(pattern.toString(), that.pattern.toString());
Review comment:
I think the hashCode should use the toString as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jon-wei merged pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
jon-wei merged pull request #9830:
URL: https://github.com/apache/druid/pull/9830
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #9830:
URL: https://github.com/apache/druid/pull/9830#issuecomment-624734779
> @suneet-s it looks like some tests are failing because Filter.equals is not always implemented. IIRC, nothing depended on that being implemented prior to this patch.
Ha good job tests! I was wondering how I'd get enough coverage to catch issues like this 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm edited a comment on pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
gianm edited a comment on pull request #9830:
URL: https://github.com/apache/druid/pull/9830#issuecomment-624730282
@suneet-s it looks like some tests are failing because Filter.equals is not always implemented. IIRC, nothing depended on that being implemented prior to this patch.
> The result of this bug is that as of 0.18 filters in the outer groupBy are ignored.
AFAICT the queries affected are groupBy stacks where the inner query is on a join datasource whose base datasource is not a table. (Join because the bug is in the join filter preanalysis code; not-a-table because it gets triggered when filter preanalysis is done based on the wrong stacked query, but Historicals don't see stacked queries, so if it's a table this wouldn't happen.) So we should add a test for this case. You couldn't do these kinds of queries at all prior to 0.18.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #9830:
URL: https://github.com/apache/druid/pull/9830#issuecomment-645083196
Tagged as Release Notes since this functionality and the fix for #9792 in #10015 depends on correct implementations of equals and hashcode for `ExtractionFn` which is an `ExtensionPoint`. Any devs of `ExtensionPoint` should also update their implementations to correctly implement equals and hashCode
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on pull request #9830: Fail incorrectly constructed join queries
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #9830:
URL: https://github.com/apache/druid/pull/9830#issuecomment-624730282
@suneet-s it looks like some tests are failing because Filter.equals is not always implemented. IIRC, nothing depended on that being implemented prior to this patch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org