julianhyde commented on code in PR #3206:
URL: https://github.com/apache/calcite/pull/3206#discussion_r1218721277
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -303,6 +303,17 @@ public interface SqlConformance {
*/
boolean isInsertSubsetColumnsAllowed();
+ /**
+ * Whether the dialect is able to ORDER BY literal values.
+ *
+ * This flag is used to filter out literal values in the
+ * order by clause of dialects that don't support them.
+ *
+ * Returns false if the dialect errors when a literal
+ * value is present in ORDER BY, true otherwise.
+ */
Review Comment:
Add a 'Default is false; true in BigQuery' paragraph similar to the other
methods in this class.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -303,6 +303,17 @@ public interface SqlConformance {
*/
boolean isInsertSubsetColumnsAllowed();
+ /**
+ * Whether the dialect is able to ORDER BY literal values.
Review Comment:
2nd ('This flag') and 3rd ('Returns false') paragraphs need to start with
`<p>`.
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -1877,6 +1885,19 @@ private SqlDialect nonOrdinalDialect() {
+ "FROM \"foodmart\".\"employee\"");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-5724";>[CALCITE-5724]
+ * Test the ORDER BY is properly removed when all literal nodes are filtered
out
+ * in a dialect that doesn't support ORDER BY literals</a>.
+ */
+ @Test void testRemoveEmptyOrderByClause() {
+ String query = "select 5, \"product_id\" from \"product\"\n"
+ + "order by 1";
+ final String expected = "SELECT 5, product_id\n"
+ + "FROM foodmart.product";
+ sql(query).withBigQuery().ok(expected);
Review Comment:
i'd chain `.withMySql().ok(expectedMySql)` just so we see what happens in
the 'normal' case.
##########
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##########
@@ -1120,6 +1120,11 @@ public List<SqlNode> fieldList() {
}
void addOrderItem(List<SqlNode> orderByList, RelFieldCollation field) {
+ // If the field being ordered on is a Literal value, we can safely skip
it.
Review Comment:
does this work if the field is an aliased literal, e.g. `date '2000-01-01'
as d`? I'd add that as a scenario to the test.
--
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]
