diff --git a/doc/changes/changes_11.0.1.md b/doc/changes/changes_11.0.1.md index 34ed04a..7871f7e 100644 --- a/doc/changes/changes_11.0.1.md +++ b/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 ` 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 diff --git a/src/main/java/com/exasol/adapter/dialects/rewriting/SqlGenerationVisitor.java b/src/main/java/com/exasol/adapter/dialects/rewriting/SqlGenerationVisitor.java index 222a152..566a718 100644 --- a/src/main/java/com/exasol/adapter/dialects/rewriting/SqlGenerationVisitor.java +++ b/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, 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 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 } 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(); diff --git a/src/test/java/com/exasol/adapter/dialects/SqlGenerationVisitorTest.java b/src/test/java/com/exasol/adapter/dialects/SqlGenerationVisitorTest.java index 843b9f3..6b9e30f 100644 --- a/src/test/java/com/exasol/adapter/dialects/SqlGenerationVisitorTest.java +++ b/src/test/java/com/exasol/adapter/dialects/SqlGenerationVisitorTest.java @@ -420,11 +420,29 @@ void testVisitSqlTableCatalogAndSchemaQualifiedQuoting() { @Test void testVisitGroupByQuoting() throws AdapterException { - final List 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 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