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

Benchao Li edited comment on CALCITE-5157 at 6/7/22 2:49 PM:
-------------------------------------------------------------

I'm ok with the fix.
I searched in the codebase, this is the only place that handles {{DOT}}'s 
operand specially, hence it's safe to fix it this way.

Other options may be:
* only generate {{SqlIdentifier}} for this case in the parser
* canonize {{DOT}} call to {{SqlIdentifier}} before validation for this case  

However, I think it's ok to fix it this way because the expression 
(SqlIdentifier DOT SqlIdentifier) is valid, and we shouldn't assume {{DOT}}'s 
first operand must be a {{SqlCall}} in this case.


was (Author: libenchao):
I'm ok with the fix.
I searched in the codebase, this is the only place that handles {{DOT}}'s 
operand specially, hence it's safe to fix it this way.

Other options may be:
* only generate {{SqlIdentifier}} for this case in the parser
* canonize {{DOT}} call to {{SqlIdentifier}} before validation for this case
However, I think it's ok to fix it this way because the expression 
(SqlIdentifier DOT SqlIdentifier) is valid, and we shouldn't assume {{DOT}}'s 
first operand must be a {{SqlCall}} in this case.

> ClassCastException in checkRollUp with DOT operator
> ---------------------------------------------------
>
>                 Key: CALCITE-5157
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5157
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.30.0
>            Reporter: Viliam Durina
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.31.0
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> When a query contains nested field access and is using parentheses to 
> disambiguate the identifier, the {{SqlValidatorImpl.checkRollup()}} method 
> throws a {{ClassCastException}}, assuming that the input of the DOT operator 
> is a {{SqlCall}}. I think this assumption is wrong, the DOT operator 
> typically has {{SqlIdentifier}} as an input, probably also other classes.
> Here's the stack trace:
> {noformat}
> java.lang.ClassCastException: class org.apache.calcite.sql.SqlIdentifier 
> cannot be cast to class org.apache.calcite.sql.SqlCall 
> (org.apache.calcite.sql.SqlIdentifier and org.apache.calcite.sql.SqlCall are 
> in unnamed module of loader 'app')
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.checkRollUp(SqlValidatorImpl.java:3730)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.checkRollUp(SqlValidatorImpl.java:3749)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.checkRollUpInSelectList(SqlValidatorImpl.java:3673)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateSelect(SqlValidatorImpl.java:3661)
>     at 
> org.apache.calcite.sql.validate.SelectNamespace.validateImpl(SelectNamespace.java:64)
>     at 
> org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:89)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:1100)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:1071)
>     at org.apache.calcite.sql.SqlSelect.validate(SqlSelect.java:247)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.validateScopedExpression(SqlValidatorImpl.java:1046)
>     at 
> org.apache.calcite.sql.validate.SqlValidatorImpl.validate(SqlValidatorImpl.java:752)
>     at 
> org.apache.calcite.sql2rel.SqlToRelConverter.convertQuery(SqlToRelConverter.java:587)
>     at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:257)
>     at org.apache.calcite.prepare.Prepare.prepareSql(Prepare.java:220)
>     at 
> org.apache.calcite.prepare.CalcitePrepareImpl.prepare2_(CalcitePrepareImpl.java:648)
>     at 
> org.apache.calcite.prepare.CalcitePrepareImpl.prepare_(CalcitePrepareImpl.java:514)
>     at 
> org.apache.calcite.prepare.CalcitePrepareImpl.prepareSql(CalcitePrepareImpl.java:484)
>     at 
> org.apache.calcite.jdbc.CalciteConnectionImpl.parseQuery(CalciteConnectionImpl.java:234)
>     at 
> org.apache.calcite.jdbc.CalciteMetaImpl.prepareAndExecute(CalciteMetaImpl.java:623)
>     at 
> org.apache.calcite.avatica.AvaticaConnection.prepareAndExecuteInternal(AvaticaConnection.java:677)
>     at 
> org.apache.calcite.avatica.AvaticaStatement.executeInternal(AvaticaStatement.java:156)
>     ... 67 more
> {noformat}
> The problem can be reproduced by modifying the 
> {{ReflectiveSchemaTest.testSelectWithFieldAccessOnFirstLevelRecordType()}} 
> test and putting {{au."birthPlace"}} into parentheses:
> {code:sql}
> select (au."birthPlace")."city" as city from ... 
> {code}
> Putting identifiers into parentheses is common to disambiguate field access 
> from identifier qualification. For example, if the {{au}} prefix is removed 
> from the query in the {{testSelectWithFieldAccessOnFirstLevelRecordType}} 
> test, it will fail with {{{}Table 'birthPlace' not found{}}}. But if we put 
> {{\"birthPlace\"}} into parentheses, then it is correctly recognized as a 
> column of the {{authors}} table.
> I'm not sure about the correct fix to this issue which would not break the 
> roll-up functionality, perhaps its author [~zhumayun] can help reviewing the 
> PR or suggesting another fix. I'll soon create a PR with a proposed fix.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to