[jira] [Commented] (CALCITE-5416) RelToSql converter generates invalid code when merging rollup and sort clauses

["Jiajun Xie (Jira)" <ji...@apache.org>]

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

Jiajun Xie commented on CALCITE-5416:
-------------------------------------

[~lchistov1987] , maybe it is expected.

There are a unit test in RelToSqlConverterTest[1].
{code:java}
/** As {@link #testSelectQueryWithGroupByRollup()},
 * but ORDER BY columns reversed. */
@Test void testSelectQueryWithGroupByRollup2() {
  final String query = "select \"product_class_id\", \"brand_name\"\n"
      + "from \"product\"\n"
      + "group by rollup(\"product_class_id\", \"brand_name\")\n"
      + "order by 2, 1";
  final String expected = "SELECT \"product_class_id\", \"brand_name\"\n"
      + "FROM \"foodmart\".\"product\"\n"
      + "GROUP BY ROLLUP(\"product_class_id\", \"brand_name\")\n"
      + "ORDER BY \"brand_name\", \"product_class_id\"";
  final String expectedMysql = "SELECT `product_class_id`, `brand_name`\n"
      + "FROM `foodmart`.`product`\n"
      + "GROUP BY `brand_name`, `product_class_id` WITH ROLLUP";
  sql(query)
      .ok(expected)
      .withMysql().ok(expectedMysql);
} {code}
Because the code in RelToSqlConverter[2].
{code:java}
if (hasTrickyRollup(e, aggregate)) {
  // MySQL 5 does not support standard "GROUP BY ROLLUP(x, y)", only
  // the non-standard "GROUP BY x, y WITH ROLLUP".
  // It does not allow "WITH ROLLUP" in combination with "ORDER BY",
  // but "GROUP BY x, y WITH ROLLUP" implicitly sorts by x, y,
  // so skip the ORDER BY.
  final Set<Integer> groupList = new LinkedHashSet<>();
  for (RelFieldCollation fc : e.collation.getFieldCollations()) {
    groupList.add(aggregate.getGroupSet().nth(fc.getFieldIndex()));
  }
  groupList.addAll(Aggregate.Group.getRollup(aggregate.getGroupSets()));
  final Builder builder =
      visitAggregate(aggregate, ImmutableList.copyOf(groupList),
          Clause.GROUP_BY, Clause.OFFSET, Clause.FETCH);
  offsetFetch(e, builder);
  return builder.result();
} {code}
[1] [https://github.com/apache/calcite/blob/main/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L690]

[2] https://github.com/apache/calcite/blob/687dec0afcfab781f905b16e422bc03bd6b9209e/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java#L868

> RelToSql converter generates invalid code when merging rollup and sort clauses
> ------------------------------------------------------------------------------
>
>                 Key: CALCITE-5416
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5416
>             Project: Calcite
>          Issue Type: Bug
>    Affects Versions: 1.32.0
>            Reporter: Leonid Chistov
>            Priority: Minor
>
> For SQL dialects (MySQL, Hive, MsSQL) that do not support "GROUP BY ROLLUP(...)" syntax, but do support "GROUP BY ... WITH ROLLUP" syntax instead, wrong code is generated by RelToSqlConverter in the following situation: 
>  * There is an Aggregate node with ROLLUP grouping
>  * It has a parent Sort node with an order of fields different from the order of fields in ROLLUP Aggregation
> This can be demonstrated by the following test, that would fail if added to RelToSqlConverterTest class:
> {code:java}
> @Test void testSelectQueryWithGroupByRollupOrderByReversed() {
>   final String query = "select \"product_class_id\", \"brand_name\"\n"
>       + "from \"product\"\n"
>       + "group by rollup(\"product_class_id\", \"brand_name\")\n"
>       + "order by 2, 1";
>   final String expectedMysql = "SELECT `product_class_id`, `brand_name`\n"
>       + "FROM `foodmart`.`product`\n"
>       + "GROUP BY `product_class_id`, `brand_name` WITH ROLLUP";
>   sql(query)
>       .withMysql().ok(expectedMysql);
> }
>  {code}
> As the result we get the following SQL code:
> {code:java}
> SELECT `product_class_id`, `brand_name
> FROM `foodmart`.`product
> GROUP BY `brand_name`, `product_class_id` WITH ROLLUP {code}
> It can be observed that order of fields of aggregation was changed to match the order of fields in ORDER clause, thus changing the semantics of the ROLLUP clause itself.



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