Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22870 )

Change subject: IMPALA-14061: Calcite Planner: added Calcite rules
......................................................................


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/22870/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22870/4//COMMIT_MSG@38
PS4, Line 38: Calcite has multiple places where it
            : creates a SEARCH operator via "simplifying" the RexNodes within
            : various rules. This operator is not supported directly in Impala
            : and we need to call "expandSearch" to handle this.
> What is Calcite SEARCH operator map to Impala? Is it BETWEEN operator?
It's more like the IN operator.  I haven't done full research on this though. 
Expanding the search just gives us stuff in Impala that we know, so that was an 
easy solution.

Eventually, it might be nice either to support the translation ourselves or 
directly support SEARCH as an Impala function.


http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java:

http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@384
PS4, Line 384: ery
> What is this 100 constant means?
I forgot to bring over the comment from the CNF rules file (and delete that 
file as well, since it is no longer used).

Copied the comment over which describes this.


http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaRexBuilder.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaRexBuilder.java:

http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaRexBuilder.java@28
PS4, Line 28: import java.util.List;
            :
> nit: Separate with new line.
Done


http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaRexBuilder.java@42
PS4, Line 42: eCall.  T
> nit: something
Done


http://gerrit.cloudera.org:8080/#/c/22870/4/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/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@93
PS4, Line 93:     timeline_.markEvent("Expanded nodes");
> Timeline event usually marked at the end of step rather than at the beginni
Done


http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/LogUtil.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/util/LogUtil.java:

http://gerrit.cloudera.org:8080/#/c/22870/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/util/LogUtil.java@33
PS4, Line 33:  public static void logDebug(Object resultObject, String 
planString) {
> Can return early if LOG.isDebugEnabled() is false.
Done



--
To view, visit http://gerrit.cloudera.org:8080/22870
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6671f7ed298a18965ef0b7a5fc10f4912333a52b
Gerrit-Change-Number: 22870
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Thu, 29 May 2025 17:01:53 +0000
Gerrit-HasComments: Yes

Reply via email to