Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21575 )
Change subject: IMPALA-13221: Calcite: Enable tpcds and tpch queries ...................................................................... Patch Set 8: (10 comments) http://gerrit.cloudera.org:8080/#/c/21575/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21575/7//COMMIT_MSG@45 PS7, Line 45: - Created bypass for some extract time operators to use > nit: extra space in "time operators" Done http://gerrit.cloudera.org:8080/#/c/21575/7//COMMIT_MSG@64 PS7, Line 64: - Added the MinusToDistinct rules. These ruleis exist in Calcite and > typo: rules Done http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/pom.xml File java/calcite-planner/pom.xml: http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/pom.xml@55 PS7, Line 55: <scope>provided</scope> > Where is this provided from? I think I did a cut and paste from the Calcite project. I removed this and it still compiles fine. http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/codegen/templates/Parser.jj File java/calcite-planner/src/main/codegen/templates/Parser.jj: http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/codegen/templates/Parser.jj@4708 PS7, Line 4708: <QUOTED_IDENTIFIER> { > How does this differ from BIG_QUERY_DOUBLE_QUOTED_STRING? It doesn't. Combined the sections http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java: http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@85 PS7, Line 85: .build(); > nit: extra whitespace Done http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java: http://gerrit.cloudera.org:8080/#/c/21575/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java@46 PS7, Line 46: * resolved. TODO: IMPALA-13022: change this comment when implicit conversion is handled. > this comment should be updated Done http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ExtractLiteralAgg.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ExtractLiteralAgg.java: http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ExtractLiteralAgg.java@43 PS8, Line 43: * to check for existence in a subquery. In this case, the Aggregate will do a group > create table lineitem(l_orderkey int, l_linenumber int); I think so? To be specific, the LITERAL_AGG is created in tpcds q45, which is: SELECT ca_zip, ca_city, sum(ws_sales_price) FROM web_sales, customer, customer_address, date_dim, item WHERE ws_bill_customer_sk = c_customer_sk AND c_current_addr_sk = ca_address_sk AND ws_item_sk = i_item_sk AND (SUBSTRING(ca_zip,1,5) IN ('85669', '86197', '88274', '83405', '86475', '85392', '85460', '80348', '81792') OR i_item_id IN (SELECT i_item_id FROM item WHERE i_item_sk IN (2, 3, 5, 7, 11, 13, 17, 19, 23, 29) )) AND ws_sold_date_sk = d_date_sk AND d_qoy = 2 AND d_year = 2001 GROUP BY ca_zip, ca_city ORDER BY ca_zip, ca_city LIMIT 100; The LITERAL_AGG is created with the part of the query that says: OR i_item_id IN (SELECT i_item_id from item WHERE i_item_sk IN (...) In answer to your second question, I did indeed test that query with this commit and it does seem to work. There is at least one query in the test framework that currently fails that will succeed with the new Calcite framework, and this seems to fit as well. http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaMinusToDistinctRule.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaMinusToDistinctRule.java: http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaMinusToDistinctRule.java@18 PS8, Line 18: package org.apache.calcite.rel.rules; > For some reason this location causes the of() method to be generated. Changed the DEFAULT variable and it worked, thanks! http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaMinusToDistinctRule.java@206 PS8, Line 206: Config DEFAULT = ImmutableImpalaMinusToDistinctRule.Config.of() > This could avoid some of the extra bits as Perfect, thanks! http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java: http://gerrit.cloudera.org:8080/#/c/21575/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@132 PS8, Line 132: JOIN_CONDITION_PUSH > CoreRules.JOIN_PUSH_TRANSITIVE_PREDICATES maybe useful for infer predictes? I'd like to hold off on adding rules for now. I did have to add some rules but they were only to get tpcds and tpch to work. I do have a future commit where I add a bunch of rules. One thing I'll have to check though: I think I had some issues with this specific rule as I did not see it in my commit. I think it might have caused an infinite loop? So if you're ok with it, I'll hold off on this one. -- To view, visit http://gerrit.cloudera.org:8080/21575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3107d336ac07ecd89530b640165798ec6a574f41 Gerrit-Change-Number: 21575 Gerrit-PatchSet: 8 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Anonymous Coward (816) Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Wed, 09 Oct 2024 22:51:13 +0000 Gerrit-HasComments: Yes
