libenchao commented on code in PR #3057:
URL: https://github.com/apache/calcite/pull/3057#discussion_r1111393434
##########
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:
No, what I mean is that we only takes pure literal as a field reference in
Calcite, if it's an expression, it won't be taken as a field reference. Your
second case shows what I described above.
To be more precise, `3 is null` equals `false`, not `count(*) is null` in
this case.
I'm not saying Calcite is definitely correct for current behavior. We need
to investigate what other databases behave when the literal is in a expression,
and let Calcite be consistent with the majority of them.
If we prefer to not take literal in expressions as a field reference, then
your change now would make it incorrect.
--
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]
