#149: Workaround for GROUP BY <integer>

doc/changes/changes_11.0.1.md

@@ -1,9 +1,21 @@
-# Virtual Schema Common JDBC 11.0.1, released 2023-??-??
+# Virtual Schema Common JDBC 11.0.1, released 2023-07-10
 
-Code name:
+Code name: Fix Issue With Integer Constants in `GROUP BY`
 
 ## Summary
 
+This release fixes an issue with queries using `DISTINCT` or `GROUP BY` with integer constants. Exasol interprets integers in `GROUP BY` clauses as column numbers which could lead to invalid results or the following error:
+
+```
+42000:Wrong column number. Too small value 0 as select list column reference in GROUP BY (smallest possible value is 1)
+```
+
+To fix this, VSCJ now replaces integer constants in `GROUP BY` clauses with a constant string. Please note that `GROUP BY <column-number>` is not affected by this and continues to work as expected.
+
+## Bugfixes
+
+* #149: Fixed issue with integer constants in `GROUP BY`
+
 ## Documentation
 
 * #147: Removed duplicate documentation about logging

src/main/java/com/exasol/adapter/dialects/rewriting/SqlGenerationVisitor.java

@@ -3,6 +3,7 @@
 import java.text.DecimalFormat;
 import java.text.NumberFormat;
 import java.util.*;
+import java.util.logging.Logger;
 
 import com.exasol.adapter.AdapterException;
 import com.exasol.adapter.adapternotes.ColumnAdapterNotesJsonConverter;
@@ -32,6 +33,7 @@
  * the right semantic, but hopefully there is a better way.
  */
 public class SqlGenerationVisitor implements SqlNodeVisitor<String>, SqlGenerator {
+    private static final Logger LOGGER = Logger.getLogger(SqlGenerationVisitor.class.getName());
     private final SqlDialect dialect;
     private final SqlGenerationContext context;
 
@@ -240,11 +242,31 @@ public String visit(final SqlFunctionAggregateListagg sqlFunctionAggregateListag
     public String visit(final SqlGroupBy groupBy) throws AdapterException {
         final List<String> selectElement = new ArrayList<>();
         for (final SqlNode node : groupBy.getExpressions()) {
-            selectElement.add(node.accept(this));
+            final SqlNode replacement = replaceGroupByExpression(node);
+            selectElement.add(replacement.accept(this));
         }
         return String.join(", ", selectElement);
     }
 
+    /**
+     * Replace an unsupported expression in a {@code GROUP BY} clause with a supported one or return it unchanged.
+     * 
+     * @implNote This replaces numeric literals with their string value as Exasol interprets
+     *           {@code GROUP BY <integer-constant>} as column numbers which was not intended.
+     * 
+     * @param node the original {@code GROUP BY} expression
+     * @return an new, alternative expression or the original expression if no replacement is necessary
+     */
+    private SqlNode replaceGroupByExpression(final SqlNode node) {
+        if (node instanceof SqlLiteralExactnumeric) {
+            final SqlLiteralExactnumeric numericNode = (SqlLiteralExactnumeric) node;
+            LOGGER.fine(() -> "Replacing numeric literal " + numericNode.getValue() + " with a string in GROUP BY");
+            return new SqlLiteralString(String.valueOf(numericNode.getValue()));
+        } else {
+            return node;
+        }
+    }
+
     @Override
     public String visit(final SqlFunctionAggregate function) throws AdapterException {
         final StringBuilder builder = new StringBuilder();

src/test/java/com/exasol/adapter/dialects/SqlGenerationVisitorTest.java

@@ -420,11 +420,29 @@ void testVisitSqlTableCatalogAndSchemaQualifiedQuoting() {
 
     @Test
     void testVisitGroupByQuoting() throws AdapterException {
-        final List<SqlNode> expressions = new ArrayList<>();
-        expressions.add(new SqlColumn(1, ColumnMetadata.builder().name("\" a '").type(DataType.createBool()).build()));
-        expressions.add(new SqlColumn(2, ColumnMetadata.builder().name("\" b '").type(DataType.createBool()).build()));
+        verifyGroupBy(List.of( //
+                new SqlColumn(1, ColumnMetadata.builder().name("\" a '").type(DataType.createBool()).build()), //
+                new SqlColumn(2, ColumnMetadata.builder().name("\" b '").type(DataType.createBool()).build())), //
+                "\"\"\" a '\", \"\"\" b '\"");
+    }
+
+    @Test
+    void testVisitGroupByReplacesIntegerLiterals() throws AdapterException {
+        verifyGroupBy(List.of( //
+                new SqlColumn(1, ColumnMetadata.builder().name("a").type(DataType.createBool()).build()), //
+                new SqlLiteralExactnumeric(BigDecimal.valueOf(42))), //
+                "\"a\", '42'");
+    }
+
+    @Test
+    void testVisitGroupByDoesNotModifyStringLiterals() throws AdapterException {
+        verifyGroupBy(List.of(new SqlLiteralString("literal")), //
+                "'literal'");
+    }
+
+    private void verifyGroupBy(final List<SqlNode> expressions, final String expectedSql) throws AdapterException {
         final SqlGroupBy groupBy = new SqlGroupBy(expressions);
-        assertThat(sqlGenerationVisitor.visit(groupBy), equalTo("\"\"\" a '\", \"\"\" b '\""));
+        assertThat(sqlGenerationVisitor.visit(groupBy), equalTo(expectedSql));
     }
 
     @Test