[
https://issues.apache.org/jira/browse/CALCITE-7120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18011622#comment-18011622
]
Steve Carlin commented on CALCITE-7120:
---------------------------------------
So I'm not a Calcite expert by any means, but what you are saying isn't
clicking with me at all.
Every database I've encountered has a tinyint that handles -128 to 127, a
smallint range, an int range, and a big int range. And even Java understands
this concept, as highlighted in the code I mention above. I don't have
arguments about the SqlNumericLiteral, which can treat this as one type. But
we're talking about the RexLiteral created at this point which is equivalent to
the database level which gets assigned a sized type. Equivalent to Java byte,
short, int, long.
I've already tried configuring it through the parser. It is not the parser's
job to derive the RelDataType. So it can't be done there. I tried a hack
(emphasize hack) to create my own derived class off of SqlNumericLiteral and
that had issues because of some static methods in validation (can't remember
which, I tried this a year ago)
The RelDataType, as far as I've seen is something that is calculated during
validation. It is buried in the SqlNumericLiteral.createSqlType() structure
which is called many times during validation. So the parser isn't really an
option.
And let me be clear about my thoughts on this. This is the validator's job.
The validator is supposed to pick the right RelDataType for everything so that
when the RelNodes are created, they have the correct types. We allow type
coercion to fix these issues and this is done in the Validator step. I don't
see how this is a RelNode layer step.
But having said that, my current hack workaround in 1.37 is to do it in the
RelNode layer. And I put extreme emphasis on the words hack workaround.
Because it got broken in 1.40 when the INTERSECT feature started creating
Projects underneath that create wider data types (I can go into more detail
perhaps, but see how the RelNodes are created with SELECT 1 INTERSECT SELECT
tinyint_col from mytbl)) Now, for me to fix my problem in the RelNode layer, I
have to find Intersect (and other) RelNodes, check to see if there are literals
that are of the wrong size compared with other RelNode branches, and fix them.
This is ugly and cumbersome, imo.
Especially when I have a 4 line fix above that fixes my problem in the
validator. When a problem can be fixed with 4 lines compared to many, usually
the 4 line solution is the correct one, imo.
But even of more important note: WE ALREADY DO THIS! We theoretically could
shove everything into a BIGINT. But we don't. We check to see if it fits into
an INTEGER based on range and if it does, we assign it an INTEGER. If it
doesn't, we assign it a BIGINT. I'm just saying we do 2 more steps to this,
which pretty much matches what most databases have!!!!
Maybe I should bring this up in the dev group for more general discussion.
> Allow SqlNumericLiteral to create more restrictive integer types
> ----------------------------------------------------------------
>
> Key: CALCITE-7120
> URL: https://issues.apache.org/jira/browse/CALCITE-7120
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Steve Carlin
> Priority: Major
>
> It would be nice if SqlNumericLiteral created more restrictive datatypes for
> integers.
> There is already some logic in there that differentiates between INTEGER and
> BIGINT
>
> {code:java}
> if ((l >= Integer.MIN_VALUE) && (l <= Integer.MAX_VALUE)) {
> result = SqlTypeName.INTEGER;
> } else {
> result = SqlTypeName.BIGINT;
> }
> {code}
> If we can enhance this for TINYINT and SMALLINT, oh how wonderful that would
> be for me.
> Background: Without this, it is causing me to use various workarounds by
> overriding methods that are less than ideal. Upon upgrade from 1.37 to 1.40,
> my current implementation failed. A query such as ...
> "SELECT 1 INTERSECT SELECT tinyint_col FROM my_tbl"
> ... is generating a validated SQLNode tree which casts the tinyint_col to an
> INTEGER (when using type coercing) which causes me issues. (side note, I
> need type coercing enabled for other issues so I can't just turn it off)
> Should we do this via a config option? Putting this in by default will
> probably break a lot of people's code.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
