You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@calcite.apache.org by "Steve Carlin (Jira)" <ji...@apache.org> on 2023/11/19 18:29:00 UTC

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

    [ https://issues.apache.org/jira/browse/CALCITE-6122 ]


    Steve Carlin deleted comment on CALCITE-6122:
    ---------------------------------------

was (Author: scarlin):
Pretty sure it's still a problem, though I still need a unit test.  The unit test in TypeCoercion only runs it through the Validator.  This is a specific SqlToRelConverter problem because it does coerce properly, it just does so too late.

> 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)