[jira] [Commented] (CALCITE-5636) Consult type mappings when coercing types
[
https://issues.apache.org/jira/browse/CALCITE-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17762525#comment-17762525
]
Will Noble commented on CALCITE-5636:
-
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, *(8640: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,
> 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)
[jira] [Commented] (CALCITE-5636) Consult type mappings when coercing types.
[
https://issues.apache.org/jira/browse/CALCITE-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17711064#comment-17711064
]
Julian Hyde commented on CALCITE-5636:
--
I am skeptical that this is a bug. The type mappings should only apply when
going from a type name to a type. Any code that already has a type (object)
should not need to consult the mapping.
Also, don't end the summary (or commit messages) with a '.'.
> 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
>
> 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)
[jira] [Commented] (CALCITE-5636) Consult type mappings when coercing types.
[
https://issues.apache.org/jira/browse/CALCITE-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17709784#comment-17709784
]
Will Noble commented on CALCITE-5636:
-
Whoops closed the other as a duplicate. Think that was some race condition.
> 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
>
> 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)
[jira] [Commented] (CALCITE-5636) Consult type mappings when coercing types.
[
https://issues.apache.org/jira/browse/CALCITE-5636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17709671#comment-17709671
]
Dan Zou commented on CALCITE-5636:
--
Hi [~wnoble], you have created two duplicate jira CALCITE-5635 and CALCITE-5636
> 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
>
> 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)
