Bruno Volpato created CALCITE-6355: -------------------------------------- Summary: RelToSqlConverter[ORDER BY] generates an incorrect order by when NULLS LAST is used in non-projected field Key: CALCITE-6355 URL: https://issues.apache.org/jira/browse/CALCITE-6355 Project: Calcite Issue Type: Bug Components: core Affects Versions: 1.36.0 Reporter: Bruno Volpato
We are using RelToSqlConverter, and seeing issues with it generating invalid queries when using _DESC NULLS LAST,_ specifically. For example, this test query: {code:java} select "product_id" from "product" where "net_weight" is not null group by "product_id" order by MAX("net_weight") desc {code} Gets resolved correctly, with a subquery, to: {code:java} SELECT "product_id" FROM (SELECT "product_id", MAX("net_weight") AS "EXPR$1" FROM "foodmart"."product" WHERE "net_weight" IS NOT NULL GROUP BY "product_id" ORDER BY 2 DESC) AS "t3" {code} However, if I specify `desc nulls last`: {code:java} select "product_id" from "product" where "net_weight" is not null group by "product_id" order by MAX("net_weight") desc nulls last {code} It creates an invalid query (order by 2, but only one field was projected): {code:java} SELECT "product_id" FROM "foodmart"."product" WHERE "net_weight" IS NOT NULL GROUP BY "product_id" ORDER BY 2 DESC NULLS LAST {code} Trying to troubleshoot it, it appears that without the `NULLS LAST`, we have the following instance: {code:java} SqlBasicCall -> SqlNumericLiteral {code} But when including it, it gets wrapped in another call: {code:java} SqlBasicCall -> SqlBasicCall -> SqlNumericLiteral {code} So the [hasSortByOrdinal method|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java#L1938C21-L1958] ends up returning {_}false{_}, which causes `needNewSubQuery` to incorrectly report _false_ too. It appears that the best way to deal with this is by using a recursion to find numeric literals - but let me know if there are better ideas. I plan to take a stab at this since I got enough context. -- This message was sent by Atlassian Jira (v8.20.10#820010)