libenchao commented on code in PR #3057:
URL: https://github.com/apache/calcite/pull/3057#discussion_r1111227023


##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -809,14 +810,14 @@ private static String toSql(RelNode root, SqlDialect 
dialect,
         + " COUNT(*) AS \"C\"\n"
         + "FROM \"foodmart\".\"product\"\n"
         + "GROUP BY ROLLUP(\"product_class_id\", \"brand_name\")\n"
-        + "ORDER BY \"product_class_id\", \"brand_name\", COUNT(*)";
+        + "ORDER BY \"product_class_id\", \"brand_name\", 3";
     final String expectedMysql = "SELECT `product_class_id`, `brand_name`,"
         + " COUNT(*) AS `C`\n"
         + "FROM `foodmart`.`product`\n"
         + "GROUP BY `product_class_id`, `brand_name` WITH ROLLUP\n"
         + "ORDER BY `product_class_id` IS NULL, `product_class_id`,"
         + " `brand_name` IS NULL, `brand_name`,"
-        + " COUNT(*) IS NULL, COUNT(*)";
+        + " 3 IS NULL, 3";

Review Comment:
   Is `order by 3 is null` valid here? In 
[Calcite](https://github.com/apache/calcite/blob/56e9deff51942fe7673096aaf5f50544f47f1411/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L6677-L6680),
 we'll take this not a 'field reference'. I'm not sure how other databases 
behaves in this case.



##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -615,6 +615,13 @@ public SqlNode orderField(int ordinal) {
         final String strValue = ((SqlNumericLiteral) node).toValue();
         return SqlLiteral.createCharString(strValue, node.getParserPosition());
       }
+      if (node instanceof SqlCall
+          && dialect.getConformance().isSortByOrdinal()) {
+        // If the filed is expression and sort by ordinal is set in dialect,

Review Comment:
   `filed` -> `field`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to