helloppx 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_r364653745
##########
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:
Thank you very much @jinxing64 , @danny0405
I did some test, it will cause another issue[1] to replace `order by
ordinal` with `order by alias`,
so I removed the code snippet[2]
[1]
```
//SqlImplementor#Result
//replace `order by ordinal` with `order by alias`
@Override public SqlNode orderField(int ordinal) {
final SqlNode node = field(ordinal);
if (node instanceof SqlIdentifier && ((SqlIdentifier) node).isSimple()) {
Result result = Result.this;
final String alias = SqlValidatorUtil.getAlias(selectList.get(ordinal),
-1);
return new SqlIdentifier(ImmutableList.of(alias), POS);
}
return node;
}
//The above method and [2] will produce wrong SQL
SELECT \"product_id\" * -1
FROM \"foodmart\".\"product\"
GROUP BY \"product_id\"
ORDER BY \"p\"
FETCH NEXT 10 ROWS ONLY
```
[2]
```
if (e.getInput() instanceof Sort) {
builder = x.builder(e);
} else {
builder = x.builder(e, Clause.SELECT);
}
```
----------------------------------------------------------------
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
