[jira] [Commented] (CALCITE-1573) shouldExpandIdentifiers do not be used to GROUP BY clause

["Julian Hyde (JIRA)" <ji...@apache.org>]

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

Julian Hyde commented on CALCITE-1573:
--------------------------------------

It sounds reasonable. But I think (I may be mistaken) that this issue came up before, and we solved it somehow. Can you provide a test case (probably in {{SqlValidatorTest}} or {{SqlToRelConverterTest}}) that fails if you do not make this fix? Do all tests pass if you make this fix?

> shouldExpandIdentifiers do not be used to GROUP BY clause
> ---------------------------------------------------------
>
>                 Key: CALCITE-1573
>                 URL: https://issues.apache.org/jira/browse/CALCITE-1573
>             Project: Calcite
>          Issue Type: Bug
>    Affects Versions: 1.11.0
>            Reporter: Dongming Liu
>            Assignee: Julian Hyde
>
> For SqlValidator, when {{shouldExpandIdentifiers}} is false, follow sql:
> {noformat}
> SELECT n_name AS c0, SUM(n_nationkey) AS c1
> FROM test.nation
> GROUP BY n_name
> {noformat}
> after validator, be converted to:
> {noformat}
> SELECT `n_name` AS `c0`, SUM(`n_nationkey`) AS `c1`
> FROM `test`.`nation`
> GROUP BY `nation`.`n_name`
> {noformat}
> The identifiers in SelectList DO NOT be expanded, but the identifier in Group By clause be expanded.
> The code:
> {code:title=For SelectList|borderStyle=solid}
>     // Create the new select list with expanded items.  Pass through
>     // the original parser position so that any overall failures can
>     // still reference the original input text.
>     SqlNodeList newSelectList =
>         new SqlNodeList(
>             expandedSelectItems,
>             selectItems.getParserPosition());
>     if (shouldExpandIdentifiers()) {
>       select.setSelectList(newSelectList);
>     }
>     getRawSelectScope(select).setExpandedSelectList(expandedSelectItems);
> {code}
> {code:title=For GroupBy|borderStyle=solid}
>     groupList = new SqlNodeList(expandedList, groupList.getParserPosition());
>     select.setGroupBy(groupList);
> {code}
> When SqlToRelConverter, the {{name}} will be null. The identifier(DO NOT be expanded) in nameMap is not equal to the {{expr.toString}} (be expanded)
> {code:title=addGroupExpr|borderStyle=solid}
>     public int addGroupExpr(SqlNode expr) {   // expr is `nation`.`n_name`
>       RexNode convExpr = bb.convertExpression(expr);
>       int ref = lookupGroupExpr(expr);
>       if (ref >= 0) {
>         return ref;
>       }
>       final int index = groupExprs.size();
>       groupExprs.add(expr);
>       String name = nameMap.get(expr.toString());  // name is null
>       addExpr(convExpr, name);
>       return index;
>     }
> {code}
> In my opinion, follow code may be correct to conside the Group By clause when validator.
> {code:title=For GroupBy|borderStyle=solid}
>     groupList = new SqlNodeList(expandedList, groupList.getParserPosition());
>     if (shouldExpandIdentifiers()) {
>       select.setGroupBy(groupList);
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)