[ 
https://issues.apache.org/jira/browse/IGNITE-22295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17862979#comment-17862979
 ] 

Pavel Pereslegin edited comment on IGNITE-22295 at 7/4/24 9:34 AM:
-------------------------------------------------------------------

Looks like there is no easy fix for this.

As described in the linked Calcite ticket, this issue does not affect JOIN...ON 
because when such an AST is validated, additional CASTs are added for the 
conditions.

For example:
{code:sql}
select e.empno from emp e
join (select '7369' as empno) c on e.empno = c.empno
{code}

AST will be transformed into the following
{code:sql}
SELECT E.EMPNO
FROM CATALOG.SALES.EMP AS E
INNER JOIN (SELECT '7369' AS EMPNO) AS C ON E.EMPNO = CAST(C.EMPNO AS INTEGER)
{code}

But NATURAL/USING JOIN AST doesn't have conditions, it only have a list of 
columns, and the validator currently only checks that they are Comparable.

h3. Possible solutions:

h4. A. Rewrite USING/NATURAL to ON so that the necessary type casts can be 
added to conditions.

But there is also a difference in processing unqualified column names for 
NATURAL/USING.

1. When star ( * ) is used common columns displayed once in the output.
2. When unqualified column name is specified it is wrapped into COALESCE with 
common columns from tables, e.g. 
{code:sql}
select empno from emp e
join (select 7369 as empno) c using (empno)
{code}

transformed to
{code:sql}
SELECT COALESCE(E.EMPNO, C.EMPNO) AS EMPNO
FROM CATALOG.SALES.EMP AS E
INNER JOIN (SELECT 7369 AS EMPNO) AS C USING (EMPNO)
{code}

 To keep this behavior it is required to override following methods of the 
{{SqlValidatorImpl}}: 

* List<String> usingNames(SqlJoin join)
* SqlNode expandExprFromJoin(SqlJoin join, SqlIdentifier identifier, 
SelectScope scope)

The problem is that {{expandExprFromJoin}} is a static private method that is 
called from package private {{SelectExpander}} class, and to override him it is 
required to copy-paste a lot of code from {{SqlValidatorImpl}} into 
{{IgniteSqlValidator}}.

At the same time, such a fix on the calcite side seems *unacceptable*, since 
doing such AST transformations in a validator is a bad idea (check 
[this|https://issues.apache.org/jira/browse/CALCITE-6413?focusedCommentId=17862822&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17862822]
 comment).

h4. B. Add necessary casts on the SqlToRelConverter side.

The implementation of this approach on the Ignite side seems unacceptable, for 
the same reason that too much code will need to be copy-pasted from Calcite.

On the Calcite side, at first glance, such a fix requires two modifications.

1. Add necessary casts to conditions in {{convertUsing}} method.
2. Add necessary casts to {{COALESCE}} in {{expandExprFromJoin}} method (may be 
this can be done in a separate ticket, but without cast COALESCE for 
unqualified column will not work with different types).

But this require deeper investigation.


was (Author: xtern):
Looks like there is no easy fix for this.

As described in the linked Calcite ticket, this issue does not affect JOIN...ON 
because when such an AST is validated, additional CASTs are added for the 
conditions.

For example:
{code:sql}
select e.empno from emp e
join (select '7369' as empno) c on e.empno = c.empno
{code}

AST will be transformed into the following
{code:sql}
SELECT E.EMPNO
FROM CATALOG.SALES.EMP AS E
INNER JOIN (SELECT '7369' AS EMPNO) AS C ON E.EMPNO = CAST(C.EMPNO AS INTEGER)
{code}

But NATURAL/USING JOIN AST doesn't have conditions, it only have a list of 
columns, and the validator currently only checks that they are Comparable.

h3. Possible solutions:

h4. A. Rewrite USING/NATURAL to ON so that the necessary type casts can be 
added to conditions.

But there is also a difference in processing unqualified column names for 
NATURAL/USING.

1. When star ( * ) is used common columns displayed once in the output.
2. When unqualified column name is specified it is wrapped into COALESCE with 
common columns from tables, e.g. 
{code:sql}
select empno from emp e
join (select 7369 as empno) c using (empno)
{code}

transformed to
{code:sql}
SELECT COALESCE(E.EMPNO, C.EMPNO) AS EMPNO
FROM CATALOG.SALES.EMP AS E
INNER JOIN (SELECT 7369 AS EMPNO) AS C USING (EMPNO)
{code}

 To keep this behavior it is required to override following methods of the 
{{SqlValidatorImpl}}: 

* List<String> usingNames(SqlJoin join)
* SqlNode expandExprFromJoin(SqlJoin join, SqlIdentifier identifier, 
SelectScope scope)

The problem is that {{expandExprFromJoin}} is a static private method that is 
called from package private {{SelectExpander}} class, and to override him it is 
required to copy-paste a lot of code from {{SqlValidatorImpl}} into 
{{IgniteSqlValidator}}.

At the same time, such a fix on the calcite side seems *unacceptable*, since 
doing such AST transformations in a validator is a bad idea (check 
[this|https://issues.apache.org/jira/browse/CALCITE-6413?focusedCommentId=17862822&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17862822]
 comment).

h4. B. Add necessary casts on the SqlToRelConverter side.

The implementation of this approach on the Ignite side seems unacceptable, for 
the same reason that too much code will need to be copy-pasted from Calcite.

On the Calcite side, at first glance, such a fix requires two modifications.

1. Add necessary casts to conditions in {{convertUsing}} method.
2. Add necessary casts to {{COALESCE}} in {{expandExprFromJoin}} method (may be 
this can be done in separate ticket).

But this require deeper investigation.

> Sql. Allow comparable types for common columns in NATURAL / USING join 
> conditions
> ---------------------------------------------------------------------------------
>
>                 Key: IGNITE-22295
>                 URL: https://issues.apache.org/jira/browse/IGNITE-22295
>             Project: Ignite
>          Issue Type: Improvement
>          Components: sql
>            Reporter: Maksim Zhuravkov
>            Assignee: Pavel Pereslegin
>            Priority: Minor
>              Labels: ignite-3
>
> As type coercion for NATURAL/USING join conditions is not invoked, such 
> queries produce incorrect results when column types of common columns do not 
> match.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to