jinxing64 commented on a change in pull request #1715: [CALCITE-3662] Generate
wrong SQL when plan contains Project(Sort(Aggregate)) and aggregate field has
no alias
URL: https://github.com/apache/calcite/pull/1715#discussion_r364073568
##########
File path:
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
##########
@@ -227,8 +227,13 @@ public Result visit(Project e) {
if (isStar(e.getChildExps(), e.getInput().getRowType(), e.getRowType())) {
return x;
}
- final Builder builder =
- x.builder(e, Clause.SELECT);
+ final Builder builder;
+ if (e.getInput() instanceof Sort) {
+ builder = x.builder(e);
+ builder.clauses.add(Clause.SELECT);
+ } else {
Review comment:
I think below fix snippet here is not correct.
```
if (e.getInput() instanceof Sort) {
builder = x.builder(e);
builder.clauses.add(Clause.SELECT);
} else {
builder = x.builder(e, Clause.SELECT);
}
```
Above change apply the projects on the select list from `Sort`. If `Sort`
uses ordinal in `ORDER BY` clause, the semantics can be wrong.
Check below case
```
select "p" * (-1) from
(select "product_id" "p", sum("net_weight") "product_id"
from "product"
group by "product_id" order by 1 limit 10) A
```
It will be converted as
```
SELECT \"product_id\" * -1
FROM \"foodmart\".\"product\"
GROUP BY \"product_id\"
ORDER BY 1
FETCH NEXT 10 ROWS ONLY
```
The order of the result is obviously inconsistent with the original Sql.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
