[
https://issues.apache.org/jira/browse/CALCITE-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762525#comment-17762525
]
Will Noble edited comment on CALCITE-5636 at 9/7/23 12:48 AM:
--------------------------------------------------------------
Finally getting back to this. Hopefully this is a much better description of
the problem. It occurs when using BigQuery's catalog type map (where
{{TIMESTAMP}} maps to {{TIMESTAMP WITH LOCAL TIME ZONE}}) and
{{SqlToRelConverter}} encounters a query like this:
{code:sql}
SELECT TIMESTAMP_ADD(DATE(1900, 1, 1), INTERVAL (1) DAY) AS `ts`
{code}
The parsed {{SqlNode}} has validated type {{TIMESTAMP(0) NOT NULL}}, and it
ends up converting into the following relational expression:
{code:java}
LogicalProject(ts=[+(CAST(DATE(1900, 1, 1)):TIMESTAMP_WITH_LOCAL_TIME_ZONE(0)
NOT NULL, *(86400000:INTERVAL DAY, 1))])
{code}
which of course has converted type {{TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT
NULL}}, so the type of the {{SqlNode}} does not match the type of the
{{{}RelNode{}}}, and it triggers [this
exception|https://github.com/apache/calcite/blob/50c3edfc3d6630528ab51fe836bd50df82cc7db8/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L489].
Btw, despite the fact that a BQ {{TIMESTAMP}} is really a timestamp w/ ltz, the
{{TIMESTAMP_ADD()}} function actually returns a {{DATETIME}} object (ISO
timestamp) when the first argument is a {{DATE}}, so the validated type of the
{{SqlNode}} is actually correct, as verified by manually running the query
against BQ. My original PR was mistaken in believing that it should be
timestamp w/ ltz, as the name seems to imply. Whoops. There may actually be a
couple bugs at play here.
The reason for the discrepancy can be seen in
{{SqlValidatorImpl.validateSelectList()}}. This invokes {{expandSelectItem()}}
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L4638],
which eventually tries to derive the type of the {{TIMESTAMP_ADD()}} call
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L6555],
which must validate the operands in order to derive the type of the function
call, and in so doing coerces the type of the first operand by casting it to an
ISO timestamp. All of this results in a value in {{expandedSelectItems}} that
looks like this:
{code:sql}
TIMESTAMP_ADD(CAST(DATE(1900, 1, 1) AS TIMESTAMP(0)), INTERVAL 1 DAY) AS `ts`
{code}
[A few lines
down|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L4659]
in {{validateSelectList()}}, we invoke {{validateExpr()}} on that expanded
node, and this eventually tries to derive the type of of that {{CAST}}
expression, which results in visiting the inserted {{SqlDataTypeSpec}}
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L6657],
and in so doing switches it from {{TIMESTAMP}} to {{TIMESTAMP WITH LOCAL TIME
ZONE}}, because it thinks that the {{CAST}} expression was written by the user,
and must therefore be translated according to the catalog reader.
I'm still thinking about how to handle this.
was (Author: wnoble):
Finally getting back to this. Hopefully this is a much better description of
the problem. It occurs when using BigQuery's catalog type map (where
{{TIMESTAMP}} maps to {{TIMESTAMP WITH LOCAL TIME ZONE}}) and
{{SqlToRelConverter}} encounters a query like this:
{code:sql}
SELECT TIMESTAMP_ADD(DATE(1900, 1, 1), INTERVAL (1) DAY) AS `ts`
{code}
The parsed {{SqlNode}} has validated type {{TIMESTAMP(0) NOT NULL}}, and it
ends up converting into the following relational expression:
{code:java}
LogicalProject(ts=[+(CAST(DATE(1900, 1, 1)):TIMESTAMP_WITH_LOCAL_TIME_ZONE(0)
NOT NULL, *(86400000:INTERVAL DAY, 1))])
{code}
which of course has converted type {{TIMESTAMP_WITH_LOCAL_TIME_ZONE(0) NOT
NULL}}, so the type of the {{SqlNode}} does not match the type of the
{{{}RelNode{}}}, and it triggers [this
exception|https://github.com/apache/calcite/blob/50c3edfc3d6630528ab51fe836bd50df82cc7db8/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java#L489].
Btw, despite the fact that a BQ {{TIMESTAMP}} is really a timestamp w/ ltz, the
{{TIMESTAMP_ADD()}} function actually returns a {{DATETIME}} object (ISO
timestamp) when the first argument is a {{DATE}}, so the validated type of the
{{SqlNode}} is actually correct, as verified by manually running the query
against BQ. My original PR was mistaken in believing that it should be
timestamp w/ ltz, as the name seems to imply. Whoops.
The reason for the discrepancy can be seen in
{{SqlValidatorImpl.validateSelectList()}}. This invokes {{expandSelectItem()}}
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L4638],
which eventually tries to derive the type of the {{TIMESTAMP_ADD()}} call
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L6555],
which must validate the operands in order to derive the type of the function
call, and in so doing coerces the type of the first operand by casting it to an
ISO timestamp. All of this results in a value in {{expandedSelectItems}} that
looks like this:
{code:sql}
TIMESTAMP_ADD(CAST(DATE(1900, 1, 1) AS TIMESTAMP(0)), INTERVAL 1 DAY) AS `ts`
{code}
[A few lines
down|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L4659]
in {{validateSelectList()}}, we invoke {{validateExpr()}} on that expanded
node, and this eventually tries to derive the type of of that {{CAST}}
expression, which results in visiting the inserted {{SqlDataTypeSpec}}
[here|https://github.com/apache/calcite/blob/bcf6bd8577b25c563b1c597c70704594a18ca1a3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L6657],
and in so doing switches it from {{TIMESTAMP}} to {{TIMESTAMP WITH LOCAL TIME
ZONE}}, because it thinks that the {{CAST}} expression was written by the user,
and must therefore be translated according to the catalog reader.
I'm still thinking about how to handle this.
> Consult type mappings when coercing types
> -----------------------------------------
>
> Key: CALCITE-5636
> URL: https://issues.apache.org/jira/browse/CALCITE-5636
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Will Noble
> Priority: Major
> Labels: pull-request-available
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> This is another offshoot of CALCITE-5346.
> In
> [{{AbstractTypeCoercion.implicitCast}}|https://github.com/apache/calcite/blob/2dba40e7a0a5651eac5a30d9e0a72f178bd9bff2/core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java#L713],
> Calcite may insert {{CAST}} invocations disregarding the root schema type
> map. This happens during the validation phase, so it's easy enough to read
> the type map directly in the {{AbstractTypeCoercion}} utility, although it's
> hard to say if this is an optimal solution. It may make sense to incorporate
> the type map into the {{RelDataTypeSystem}} which is more widely available in
> non-validation phases of SQL processing pipelines.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
