[ 
https://issues.apache.org/jira/browse/CALCITE-6122?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Steve Carlin reopened CALCITE-6122:
-----------------------------------

Reopening this now that I've had time to investigate further.

I have two unit tests that I added to SqlValidatorTest that showcase the 
problem:
  
{code:java}
@Test void testShouldHaveCast() {
    sql("select sum(ename) from emp")
        .withValidatorIdentifierExpansion(false)
        .rewritesTo("SELECT SUM(`ENAME`)\n"
            + "FROM `EMP`");
  }
 
  @Test void testCorrectWithCast() {
    sql("select sum(ename) from emp")
        .withValidatorIdentifierExpansion(true)
        .rewritesTo("SELECT SUM(CAST(`EMP`.`ENAME` AS DECIMAL(19, 9)))\n"
            + "FROM `CATALOG`.`SALES`.`EMP` AS `EMP`");
  }
{code}
The "identifierExpansion" config parameter has the side effect of also allowing 
type coercion to be sent back with the validated node.

I think I can live with this as a workaround for my code, but it's not great.  
I also don't know how easy this will be to fix given that it involves a mutated 
selectList parameter in SqlValidatorImpl which gets changed by the 
identifierExpansion config param.  That code can be found here (in 
SqlValidatorImpl.java):

 
{code:java}
4663     // Create the new select list with expanded items.  Pass through
4664     // the original parser position so that any overall failures can
4665     // still reference the original input text.
4666     SqlNodeList newSelectList =
4667         new SqlNodeList(expandedSelectItems, 
selectItems.getParserPosition());
4668     if (config.identifierExpansion()) {
4669       select.setSelectList(newSelectList);
4670     }
{code}
 

 

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

Reply via email to