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

Change subject: IMPALA-14429: Calcite planner: change mechanism for parsing 
tinyint, smallint
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23724/9/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/23724/9/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaRexBuilder.java@69
PS9, Line 69: type.getSqlTypeName().equals(SqlTypeName.INTEGER)
> It seems to me any exact numeric type (there are 9 such types in Calcite) c
Ok, changed it.

Yeah, it shouldn't really matter. There isn't any case right now where Calcite 
would give you any of those other exact numeric types since they always give 
you int.

But your change makes sense because I suppose they can change it AND the 
defaults could theoretically be something different from Impala.  So we should 
always use the Impala type.


http://gerrit.cloudera.org:8080/#/c/23724/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ImpalaSqlValidatorImpl.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ImpalaSqlValidatorImpl.java:

http://gerrit.cloudera.org:8080/#/c/23724/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ImpalaSqlValidatorImpl.java@54
PS4, Line 54: import java.math.BigDecimal;
> nit: an empty line after the last import would be nice
Done


http://gerrit.cloudera.org:8080/#/c/23724/4/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/ImpalaSqlValidatorImpl.java@60
PS4, Line 60:  */
> Not clear why you added a newline here.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I67b6f7711093a4b8488beee0893aea3c72239eb0
Gerrit-Change-Number: 23724
Gerrit-PatchSet: 9
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
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: Pranav Lodha <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Sun, 22 Feb 2026 15:15:07 +0000
Gerrit-HasComments: Yes

Reply via email to