Xiaoqing Gao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table ...................................................................... Patch Set 6: (8 comments) http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@7 PS5, Line 7: applied to view table : > nit: 'applied to a view' Done http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@9 PS5, Line 9: For OR predicates that reference a view, currently the ConvertToCNFRule > This sentence seems to be a continuation of the title summary. In general, Done http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@13 PS5, Line 13: suc > nit: 'Added' Done http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java: http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@122 PS5, Line 122: if (tbl instanceof InlineViewRef) { > tids.get(0) could hit index out of bounds if tids was empty .. which is a c Done http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@124 PS5, Line 124: analyzer, false); > I think this should use 'cpred' instead of pred since cpred is the the anal Done http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test: http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2487 PS5, Line 2487: appli > nit: 'applied' Done http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2491 PS5, Line 2491: select count(*) from t where t.test_zip = 1 or (t.test_name='xyz' and t.zip=1); > Could you also add a test where the inline view itself is joined with anoth Done http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2515 PS5, Line 2515: # IMPALA-10412: ConvertToCNFRule can be applied to view table > It wasn't clear what the purpose of this test was. If it is not really exer Done -- To view, visit http://gerrit.cloudera.org:8080/16912 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88 Gerrit-Change-Number: 16912 Gerrit-PatchSet: 6 Gerrit-Owner: Xiaoqing Gao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Xiaoqing Gao <[email protected]> Gerrit-Comment-Date: Mon, 04 Jan 2021 08:12:12 +0000 Gerrit-HasComments: Yes
