[CALCITE-4901] JDBC adapter incorrectly adds ORDER BY columns to the …

…SELECT list of generated SQL query

Close apache#2665

core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java

@@ -431,7 +431,13 @@ public Result visit(Filter e) {
 
   /** Visits a Project; called by {@link #dispatch} via reflection. */
   public Result visit(Project e) {
-    final Result x = visitInput(e, 0, Clause.SELECT);
+    // If the input is a Sort, wrap SELECT is not required.
+    final Result x;
+    if (e.getInput() instanceof Sort) {
+      x = visitInput(e, 0);
+    } else {
+      x = visitInput(e, 0, Clause.SELECT);
+    }
     parseCorrelTable(e, x);
     final Builder builder = x.builder(e);
     if (!isStar(e.getProjects(), e.getInput().getRowType(), e.getRowType())) {

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

@@ -221,6 +221,23 @@ private static String toSql(RelNode root, SqlDialect dialect,
     sql(query).ok(expected);
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-4901">[CALCITE-4901]
+   * JDBC adapter incorrectly adds ORDER BY columns to the SELECT list</a>. */
+  @Test void testOrderByNotInSelectList() {
+    // Before 4901 was fixed, the generated query would have "product_id" in its
+    // SELECT clause.
+    String query = "select count(1) as c\n"
+        + "from \"foodmart\".\"product\"\n"
+        + "group by \"product_id\"\n"
+        + "order by \"product_id\" desc";
+    final String expected = "SELECT COUNT(*) AS \"C\"\n"
+        + "FROM \"foodmart\".\"product\"\n"
+        + "GROUP BY \"product_id\"\n"
+        + "ORDER BY \"product_id\" DESC";
+    sql(query).ok(expected);
+  }
+
   @Test void testAggregateFilterWhereToSqlFromProductTable() {
     String query = "select\n"
         + "  sum(\"shelf_width\") filter (where \"net_weight\" > 0),\n"
@@ -1516,7 +1533,7 @@ private void checkHavingAliasSameAsColumn(boolean upperAlias) {
   @Test void testSelectQueryWithOrderByClause() {
     String query = "select \"product_id\" from \"product\"\n"
         + "order by \"net_weight\"";
-    final String expected = "SELECT \"product_id\", \"net_weight\"\n"
+    final String expected = "SELECT \"product_id\"\n"
         + "FROM \"foodmart\".\"product\"\n"
         + "ORDER BY \"net_weight\"";
     sql(query).ok(expected);
@@ -1534,8 +1551,7 @@ private void checkHavingAliasSameAsColumn(boolean upperAlias) {
   @Test void testSelectQueryWithTwoOrderByClause() {
     String query = "select \"product_id\" from \"product\"\n"
         + "order by \"net_weight\", \"gross_weight\"";
-    final String expected = "SELECT \"product_id\", \"net_weight\","
-        + " \"gross_weight\"\n"
+    final String expected = "SELECT \"product_id\"\n"
         + "FROM \"foodmart\".\"product\"\n"
         + "ORDER BY \"net_weight\", \"gross_weight\"";
     sql(query).ok(expected);
@@ -1544,8 +1560,7 @@ private void checkHavingAliasSameAsColumn(boolean upperAlias) {
   @Test void testSelectQueryWithAscDescOrderByClause() {
     String query = "select \"product_id\" from \"product\" "
         + "order by \"net_weight\" asc, \"gross_weight\" desc, \"low_fat\"";
-    final String expected = "SELECT"
-        + " \"product_id\", \"net_weight\", \"gross_weight\", \"low_fat\"\n"
+    final String expected = "SELECT \"product_id\"\n"
         + "FROM \"foodmart\".\"product\"\n"
         + "ORDER BY \"net_weight\", \"gross_weight\" DESC, \"low_fat\"";
     sql(query).ok(expected);
@@ -2604,13 +2619,13 @@ private void checkHavingAliasSameAsColumn(boolean upperAlias) {
   @Test void testSelectQueryWithLimitOffsetClause() {
     String query = "select \"product_id\" from \"product\"\n"
         + "order by \"net_weight\" asc limit 100 offset 10";
-    final String expected = "SELECT \"product_id\", \"net_weight\"\n"
+    final String expected = "SELECT \"product_id\"\n"
         + "FROM \"foodmart\".\"product\"\n"
         + "ORDER BY \"net_weight\"\n"
         + "OFFSET 10 ROWS\n"
         + "FETCH NEXT 100 ROWS ONLY";
     // BigQuery uses LIMIT/OFFSET, and nulls sort low by default
-    final String expectedBigQuery = "SELECT product_id, net_weight\n"
+    final String expectedBigQuery = "SELECT product_id\n"
         + "FROM foodmart.product\n"
         + "ORDER BY net_weight IS NULL, net_weight\n"
         + "LIMIT 100\n"
@@ -6463,7 +6478,7 @@ String exec() {
               getPlanner(null, parserConfig, defaultSchema, config, librarySet, typeSystem);
           SqlNode parse = planner.parse(sql);
           SqlNode validate = planner.validate(parse);
-          rel = planner.rel(validate).rel;
+          rel = planner.rel(validate).project();
         }
         for (Function<RelNode, RelNode> transform : transforms) {
           rel = transform.apply(rel);

piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java

@@ -479,14 +479,14 @@ private Fluent pig(String script) {
         + "HIREDATE, SAL, COMM, DEPTNO)) AS A\n"
         + "        FROM scott.EMP\n"
         + "        GROUP BY DEPTNO) AS $cor4,\n"
-        + "      LATERAL (SELECT COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n"
-        + "        FROM (SELECT ENAME, JOB, DEPTNO, SAL\n"
+        + "      LATERAL (SELECT X\n"
+        + "        FROM (SELECT 'all' AS $f0, COLLECT(ROW(ENAME, JOB, DEPTNO, SAL)) AS X\n"
         + "            FROM UNNEST (SELECT $cor4.A AS $f0\n"
         + "                FROM (VALUES (0)) AS t (ZERO)) "
         + "AS t2 (EMPNO, ENAME, JOB, MGR, HIREDATE, SAL, COMM, DEPTNO)\n"
         + "            WHERE JOB <> 'CLERK'\n"
-        + "            ORDER BY SAL) AS t5\n"
-        + "        GROUP BY 'all') AS t8) AS $cor5,\n"
+        + "            GROUP BY 'all'\n"
+        + "            ORDER BY SAL) AS t7) AS t8) AS $cor5,\n"
         + "  LATERAL UNNEST (SELECT $cor5.X AS $f0\n"
         + "    FROM (VALUES (0)) AS t (ZERO)) "
         + "AS t11 (ENAME, JOB, DEPTNO, SAL) AS t110\n"