[jira] [Comment Edited] (CALCITE-6122) In SqlToRelConverter, AggConverter doesn't use coerced SqlNodes

["Tanner Clary (Jira)" <ji...@apache.org>]

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

Tanner Clary edited comment on CALCITE-6122 at 11/17/23 3:51 PM:
-----------------------------------------------------------------

As you might have found, [{{SqlToRelConverterTest}}|https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java] may provide some inspiration for writing a unit test // some examples. 


was (Author: JIRAUSER298151):
As you might have found, {{SqlToRelConverterTest}} may provide some inspiration for writing a unit test // some examples. 

> In SqlToRelConverter, AggConverter doesn't use coerced SqlNodes
> ---------------------------------------------------------------
>
>                 Key: CALCITE-6122
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6122
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.36.0
>            Reporter: Steve Carlin
>            Priority: Major
>
> I hope I'm describing this right.
> I'm coercing an operand in my handmade OperandTypeChecker. Specifically, I'm changing a {color:#de350b}SUM(tinyint_col){color} to a {color:#de350b}SUM(CAST(tinyint_col as BIGINT)){color} because my database can only handle a bigint operand.
> The eventual logical plan does not keep the CAST operand.
> I had this problem in 1.34.0.  I noticed that the code changed quite a bit going up to 1.36.0, so I say this to mention that this is not a regression.
> I hacked a fix in my environment, but it's too hacky to commit.  To fix the problem, I changed to the following code in SqlToRelConverter.convertAgg():
>  
> {code:java}
> @@ -3369,7 +3372,11 @@ protected void convertAgg(Blackboard bb, SqlSelect select,
>      final AggConverter aggConverter =
>          AggConverter.create(bb,
>              (AggregatingSelectScope) validator().getSelectScope(select));
> -    createAggImpl(bb, aggConverter, selectList, groupList, having,
> +    selectList.accept(aggConverter);
> +    final AggConverter aggConverter2 =
> +        AggConverter.create(bb,
> +            (AggregatingSelectScope) validator().getSelectScope(select));
> +    createAggImpl(bb, aggConverter2, selectList, groupList, having,
>          orderExprList);
>    }
> {code}
> Note that I had the selectList go through the aggConverter visitor.  After this, the selectList contains the coerced operands.  If the aggConverter is created based on this new selectList, it will contain the proper information in the aggConverter.convertedInputExprs list
> (One other note: There is an assertion in createAggImpl that i had to disable in order to get this hack to work where it checks that bb.agg == null)
> I can probably work on this, but I'm not sure how to create a proper test for it, as I've never committed anything to Calcite before.
>  



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