Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/21688 )
Change subject: IMPALA-13262: Do not always migrate inferred predicates into inline view ...................................................................... Patch Set 2: (4 comments) Hi all, I have addressed some of reviewers' comments on the previous patch set. We may need some more discussion around https://gerrit.cloudera.org/c/21688/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java#1567. Let me know what you think about it. Thanks! http://gerrit.cloudera.org:8080/#/c/21688/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java: http://gerrit.cloudera.org:8080/#/c/21688/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1528 PS1, Line 1528: for (Expr e: viewPredicates) e.setIsOnClauseConjunct(false); > nit: we could pass this in from migrateConjunctsToInlineView -> migrateOrCo Thanks Michael! I will adopt Riza's suggestion in the next patch set. Let me know if you have any concern. http://gerrit.cloudera.org:8080/#/c/21688/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1527 PS1, Line 1527: // apply to the post-join/agg/analytic result of the inline view. : for (Expr e: viewPredicates) e.setIsOnClauseConjunct(fals > This is the only callsite to removeDisqualifyingInferredPreds(). The functi Thanks I will revise the patch accordingly. http://gerrit.cloudera.org:8080/#/c/21688/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1567 PS1, Line 1567: or right side i > It is probably enough to check for equality between leftParent and rightPar Thanks Riza! I have a related question in the following regarding your suggestion above. Is it true that for an inferred binary predicate, if 'leftParent' is equal to 'rightParent', it's guaranteed that both parents will resolve to the same base table (v.s. the same inline view or the same intermediate analytic tuple)? If that's the case then we should be able to use 'leftParent.equals(rightParent)' instead. I have not been able to come up with a counter example where both parents resolve to the same non-base table. The closest one is the following test case added in IMPALA-11843 from https://gerrit.cloudera.org/c/19422/2/testdata/workloads/functional-query/queries/QueryTest/partitioned-top-n.test#123. In this specific test case, 'rightParent' (of max(id)) corresponds to an intermediate analytic tuple, where 'leftParent' (of the column 'id') resolves to the table 'functional.alltypesagg'. select id, rn from ( select id, row_number() over (order by id desc) rn, max(id) over () max_id from functional.alltypesagg) t where id = max_id and rn < 10; On the other hand, if we don't use 'leftParent.equals(rightParent)', then we should probably perform a null check on 'leftTable' before 'leftTable.equals(rightTable)' since 'leftTable' could theoretically be null too, e.g., when it corresponds to an inline view or an intermediate analytic tuple. However, I was not able to come up with such an example even I changed the predicate from 'id = max_id' to 'max_id = id'. http://gerrit.cloudera.org:8080/#/c/21688/1/testdata/workloads/functional-query/queries/QueryTest/inline-view.test File testdata/workloads/functional-query/queries/QueryTest/inline-view.test: http://gerrit.cloudera.org:8080/#/c/21688/1/testdata/workloads/functional-query/queries/QueryTest/inline-view.test@597 PS1, Line 597: # IMPALA-13262: Do not migrate an inferred predicate into an inline view if both sides of > Please add these two test cases for output comparison: Thanks Riza! I will add these 2 test cases in the next patch set. -- To view, visit http://gerrit.cloudera.org:8080/21688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e2632b3b1a140ae0104ceba4e2f474ac1bbcda1 Gerrit-Change-Number: 21688 Gerrit-PatchSet: 2 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Tue, 20 Aug 2024 21:20:45 +0000 Gerrit-HasComments: Yes
