[
https://issues.apache.org/jira/browse/CALCITE-6122?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17787581#comment-17787581
]
Steve Carlin commented on CALCITE-6122:
---------------------------------------
So I'm not feeling great about how to fix this since I don't know the code too
well. I had a fix in 1.34.0 which worked, and it might be the way to go. I
don't feel great about changing the code because it involves the type coercion
code which mutates an object subtly underneath the covers, so it feels a bit
weird. It is line 488 in 1.36.0 in AggConverter.java:
{code:java}
final RelDataType type = validator().deriveType(bb.scope(), call);{code}
The purpose seems to be to get the derived type, but this has the effect of
mutating the call with the latest type coercions, and that is exactly what I
need for my code to work.
If I move this code before the try block on line 442 where it looks through the
call operands, it fixes my problem. But it seems more like a side effect here
to move this call up rather than the actual intention. Perhaps some who knows
this code more than I do can chime in? Not sure who that would be though. The
last person who touched this was [~julianhyde] , but it was just to move the
AggConverter class out of SqlToRelConverter.
> 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)
