Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21679 )
Change subject: IMPALA-13302: Analyze new rewrite exprs ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/21679/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/21679/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3348 PS5, Line 3348: If we're marking it now, that can cause conflicts : // with an Expr that gets assigned the same ID, usually skipping slot materialization. > I don't think it's valid to try to call this function on multiple Expr shar I think https://gerrit.cloudera.org/c/21684 is more correct. We shouldn't be calling markConjunctsAssigned on conjuncts that weren't registered. There might be other ways to fix registerConjuncts, but I think it's current implementation is broken. This patch avoids it by ensuring that - during reAnalyze - we don't encounter the same conjuncts again because they were successfully rewritten out in the first pass. - What should happen is: conjuncts are registered, then marked assigned; any later conjuncts will then get larger IDs that don't collide. - Instead: some Expr are registered in the first pass (because the statement wasn't constant, as rewrites were incomplete). In reAnalyze the statement is now constant false so we markConjunctAssigned on Expr that were registered in the first analyzer, without registering them with this analyzer; that messes up later Expr that get conflicting IDs. This patch makes more sense as a separate ticket to ensure rewrite rules are fully applied. Currently some are missed because they results don't get analyzed (and thus can't be further rewritten), and are getting fixed piecemeal. -- To view, visit http://gerrit.cloudera.org:8080/21679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6be731c2ea79c96e51d199c822e2cb34e5bb3028 Gerrit-Change-Number: 21679 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 29 Aug 2024 00:06:13 +0000 Gerrit-HasComments: Yes
