From 2e58872712ed1d0cc248d094e8af456ff47ee9ce Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Mon, 1 Jun 2026 14:22:45 +0200 Subject: [PATCH 1/6] #315: Improve PushdownSqlParser table lookup and traversal performance --- doc/changes/changes_18.0.2.md | 1 + .../request/parser/PushdownSqlParser.java | 268 +++++++++--------- .../request/parser/PushDownSqlParserTest.java | 42 ++- 3 files changed, 172 insertions(+), 139 deletions(-) diff --git a/doc/changes/changes_18.0.2.md b/doc/changes/changes_18.0.2.md index 00a63aaa..ba720994 100644 --- a/doc/changes/changes_18.0.2.md +++ b/doc/changes/changes_18.0.2.md @@ -22,6 +22,7 @@ Code name: Improve code quality * #309: Fixed the `SqlNodeVisitor` API documentation and parameter names for `SqlPredicateIsNotNull` and `SqlPredicateIsNull`. * #308: Fixed `SqlLimit` so `limit` and `offset` stay non-negative for the full object lifetime by making the node immutable and aligning the validation message with the accepted zero values. * #307: Fixed `PushdownSqlRenderer` so `HASHTYPE` data types include `bytesize` in rendered pushdown SQL JSON. +* #315: Improved `PushdownSqlParser` lookup and traversal performance by caching involved table metadata, defensively copying the metadata list, and replacing `Stack`-based traversals with `ArrayDeque`. * #312: Fixed logging utility resource and handler lifecycle issues by closing version metadata streams, falling back when the thread context class loader is missing, defaulting absent version properties to `UNKNOWN`, closing replaced root log handlers, and resetting closed remote socket handlers before reuse. ## Dependency Updates diff --git a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java index 63b578fc..cb73975b 100644 --- a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java @@ -23,9 +23,10 @@ */ public final class PushdownSqlParser extends AbstractRequestParser { private final List involvedTablesMetadata; + private Map involvedTablesMetadataMap; private PushdownSqlParser(final List involvedTablesMetadata) { - this.involvedTablesMetadata = involvedTablesMetadata; + this.involvedTablesMetadata = new ArrayList<>(involvedTablesMetadata); } private static ExaCharset charSetFromString(final String charset) { @@ -141,19 +142,23 @@ private SqlGroupBy parseGroupBy( } } - private boolean hasAggregateFunction(final List nodesList) { - // Stack is less efficient than ArrayDeque, but ArrayDeque doesn't support null elements. - @java.lang.SuppressWarnings("java:S1149") - Stack expressions = new Stack<>(); - expressions.addAll(nodesList); + static boolean hasAggregateFunction(final List nodesList) { + final Deque expressions = new ArrayDeque<>(); + for (final SqlNode node : nodesList) { + if (node != null) { + expressions.push(node); + } + } while (!expressions.isEmpty()) { final SqlNode expression = expressions.pop(); - if (expression != null) { - expressions.addAll(expression.getChildren()); - if (expression.getType().equals(SqlNodeType.FUNCTION_AGGREGATE)) { - return true; + for (final SqlNode child : expression.getChildren()) { + if (child != null) { + expressions.push(child); } } + if (expression.getType().equals(SqlNodeType.FUNCTION_AGGREGATE)) { + return true; + } } return false; } @@ -199,91 +204,93 @@ public SqlNode parseExpression(final JsonObject expression) { final String typeName = expression.getString(TYPE, ""); final SqlNodeType type = fromTypeName(typeName); switch (type) { - case SELECT: - return parseSelect(expression); - case TABLE: - return parseTable(expression); - case JOIN: - return parseJoin(expression); - case COLUMN: - return parseColumn(expression); - case LITERAL_NULL: - return parseLiteralNull(); - case LITERAL_BOOL: - return parseLiteralBool(expression); - case LITERAL_DATE: - return parseLiteralDate(expression); - case LITERAL_TIMESTAMP: - return parseLiteralTimestamp(expression); - case LITERAL_TIMESTAMPUTC: - return parseLiteralTimestamputc(expression); - case LITERAL_DOUBLE: - return parseLiteralDouble(expression); - case LITERAL_EXACTNUMERIC: - return parseLiteralExactNumeric(expression); - case LITERAL_STRING: - return parseLiteralString(expression); - case LITERAL_INTERVAL: - return parseLiteralInterval(expression); - case PREDICATE_AND: - return parsePredicateAnd(expression); - case PREDICATE_OR: - return parsePredicateOr(expression); - case PREDICATE_NOT: - return parsePredicateNot(expression); - case PREDICATE_EQUAL: - return parsePredicateEqual(expression); - case PREDICATE_NOTEQUAL: - return parsePredicateNotEqual(expression); - case PREDICATE_LESS: - return parsePredicateLess(expression); - case PREDICATE_LESSEQUAL: - return parsePredicateLessEqual(expression); - case PREDICATE_LIKE: - return parsePredicateLike(expression); - case PREDICATE_LIKE_REGEXP: - return parsePredicateLikeRegexp(expression); - case PREDICATE_BETWEEN: - return parsePredicateBetween(expression); - case PREDICATE_IN_CONSTLIST: - return parsePredicateInConstlist(expression); - case PREDICATE_IS_JSON: - return parsePredicateIsJson(expression); - case PREDICATE_IS_NOT_JSON: - return parsePredicateIsNotJson(expression); - case PREDICATE_IS_NULL: - return parsePredicateIsNull(expression); - case PREDICATE_IS_NOT_NULL: - return parsePredicateIsNotNull(expression); - case FUNCTION_SCALAR: - return parseFunctionScalar(expression); - case FUNCTION_SCALAR_EXTRACT: - return parseFunctionScalarExtract(expression); - case FUNCTION_SCALAR_CASE: - return parseFunctionScalarCase(expression); - case FUNCTION_SCALAR_CAST: - return parseFunctionScalarCast(expression); - case FUNCTION_SCALAR_JSON_VALUE: - return parseFunctionScalarJsonValue(expression); - case FUNCTION_AGGREGATE: - return parseFunctionAggregate(expression); - case FUNCTION_AGGREGATE_GROUP_CONCAT: - return parseFunctionAggregateGroupConcat(expression); - case FUNCTION_AGGREGATE_LISTAGG: - return parseFunctionAggregateListagg(expression); - default: - throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-8") // - .message("Unknown node type: {{typeName}}") // - .parameter("typeName", typeName).toString()); + case SELECT: + return parseSelect(expression); + case TABLE: + return parseTable(expression); + case JOIN: + return parseJoin(expression); + case COLUMN: + return parseColumn(expression); + case LITERAL_NULL: + return parseLiteralNull(); + case LITERAL_BOOL: + return parseLiteralBool(expression); + case LITERAL_DATE: + return parseLiteralDate(expression); + case LITERAL_TIMESTAMP: + return parseLiteralTimestamp(expression); + case LITERAL_TIMESTAMPUTC: + return parseLiteralTimestamputc(expression); + case LITERAL_DOUBLE: + return parseLiteralDouble(expression); + case LITERAL_EXACTNUMERIC: + return parseLiteralExactNumeric(expression); + case LITERAL_STRING: + return parseLiteralString(expression); + case LITERAL_INTERVAL: + return parseLiteralInterval(expression); + case PREDICATE_AND: + return parsePredicateAnd(expression); + case PREDICATE_OR: + return parsePredicateOr(expression); + case PREDICATE_NOT: + return parsePredicateNot(expression); + case PREDICATE_EQUAL: + return parsePredicateEqual(expression); + case PREDICATE_NOTEQUAL: + return parsePredicateNotEqual(expression); + case PREDICATE_LESS: + return parsePredicateLess(expression); + case PREDICATE_LESSEQUAL: + return parsePredicateLessEqual(expression); + case PREDICATE_LIKE: + return parsePredicateLike(expression); + case PREDICATE_LIKE_REGEXP: + return parsePredicateLikeRegexp(expression); + case PREDICATE_BETWEEN: + return parsePredicateBetween(expression); + case PREDICATE_IN_CONSTLIST: + return parsePredicateInConstlist(expression); + case PREDICATE_IS_JSON: + return parsePredicateIsJson(expression); + case PREDICATE_IS_NOT_JSON: + return parsePredicateIsNotJson(expression); + case PREDICATE_IS_NULL: + return parsePredicateIsNull(expression); + case PREDICATE_IS_NOT_NULL: + return parsePredicateIsNotNull(expression); + case FUNCTION_SCALAR: + return parseFunctionScalar(expression); + case FUNCTION_SCALAR_EXTRACT: + return parseFunctionScalarExtract(expression); + case FUNCTION_SCALAR_CASE: + return parseFunctionScalarCase(expression); + case FUNCTION_SCALAR_CAST: + return parseFunctionScalarCast(expression); + case FUNCTION_SCALAR_JSON_VALUE: + return parseFunctionScalarJsonValue(expression); + case FUNCTION_AGGREGATE: + return parseFunctionAggregate(expression); + case FUNCTION_AGGREGATE_GROUP_CONCAT: + return parseFunctionAggregateGroupConcat(expression); + case FUNCTION_AGGREGATE_LISTAGG: + return parseFunctionAggregateListagg(expression); + default: + throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-8") // + .message("Unknown node type: {{typeName}}") // + .parameter("typeName", typeName).toString()); } } private Map getInvolvedTablesMetadataMap() { - final Map tableMetadataMap = new HashMap<>(this.involvedTablesMetadata.size()); - for (final TableMetadata involvedTableMeta : this.involvedTablesMetadata) { - tableMetadataMap.put(involvedTableMeta.getName(), involvedTableMeta); + if (this.involvedTablesMetadataMap == null) { + this.involvedTablesMetadataMap = new HashMap<>(this.involvedTablesMetadata.size()); + for (final TableMetadata involvedTableMeta : this.involvedTablesMetadata) { + this.involvedTablesMetadataMap.put(involvedTableMeta.getName(), involvedTableMeta); + } } - return tableMetadataMap; + return this.involvedTablesMetadataMap; } private SqlColumn createColumn(final int index, final SqlTable table, final ColumnMetadata columnMetadata) { @@ -458,22 +465,22 @@ private DataType getGeometry(final JsonObject dataType) { */ private List collectInvolvedTables(final SqlNode from) { final List involvedTables = new ArrayList<>(); - final Stack nodes = new Stack<>(); - nodes.add(from); + final Deque nodes = new ArrayDeque<>(); + nodes.push(from); while (!nodes.isEmpty()) { final SqlNode node = nodes.pop(); switch (node.getType()) { - case TABLE: - involvedTables.add((SqlTable) node); - break; - case JOIN: - nodes.add(((SqlJoin) node).getRight()); - nodes.add(((SqlJoin) node).getLeft()); - break; - default: - throw new IllegalStateException(ExaError.messageBuilder("E-VSCOMJAVA-10") - .message("Encountered illegal SqlNodeType during collection involved tables: {{nodeType}}") - .parameter("nodeType", node.getType()).toString()); + case TABLE: + involvedTables.add((SqlTable) node); + break; + case JOIN: + nodes.push(((SqlJoin) node).getRight()); + nodes.push(((SqlJoin) node).getLeft()); + break; + default: + throw new IllegalStateException(ExaError.messageBuilder("E-VSCOMJAVA-10") + .message("Encountered illegal SqlNodeType during collection involved tables: {{nodeType}}") + .parameter("nodeType", node.getType()).toString()); } } return involvedTables; @@ -482,30 +489,30 @@ private List collectInvolvedTables(final SqlNode from) { private DataType getDataType(final JsonObject dataType) { final String typeName = dataType.getString(TYPE).toUpperCase(); switch (typeName) { - case "DECIMAL": - return DataType.createDecimal(dataType.getInt(PRECISION), dataType.getInt(SCALE)); - case "DOUBLE": - return DataType.createDouble(); - case "VARCHAR": - return getVarchar(dataType); - case "CHAR": - return getChar(dataType); - case "BOOLEAN": - return DataType.createBool(); - case "DATE": - return DataType.createDate(); - case "TIMESTAMP": - return getTimestamp(dataType); - case "INTERVAL": - return getInterval(dataType); - case "GEOMETRY": - return getGeometry(dataType); - case "HASHTYPE": - return getHashtype(dataType); - default: - throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-11") - .message("Unsupported data type encountered: {{typeName}}.") // - .parameter("typeName", typeName).toString()); + case "DECIMAL": + return DataType.createDecimal(dataType.getInt(PRECISION), dataType.getInt(SCALE)); + case "DOUBLE": + return DataType.createDouble(); + case "VARCHAR": + return getVarchar(dataType); + case "CHAR": + return getChar(dataType); + case "BOOLEAN": + return DataType.createBool(); + case "DATE": + return DataType.createDate(); + case "TIMESTAMP": + return getTimestamp(dataType); + case "INTERVAL": + return getInterval(dataType); + case "GEOMETRY": + return getGeometry(dataType); + case "HASHTYPE": + return getHashtype(dataType); + default: + throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-11") + .message("Unsupported data type encountered: {{typeName}}.") // + .parameter("typeName", typeName).toString()); } } @@ -725,10 +732,9 @@ private static SqlNodeType fromTypeName(final String typeName) { private TableMetadata findInvolvedTableMetadata(final String tableName) { assert this.involvedTablesMetadata != null; - for (final TableMetadata tableMetadata : this.involvedTablesMetadata) { - if (tableMetadata.getName().equals(tableName)) { - return tableMetadata; - } + final TableMetadata tableMetadata = getInvolvedTablesMetadataMap().get(tableName); + if (tableMetadata != null) { + return tableMetadata; } throw new IllegalStateException(ExaError.messageBuilder("E-VSCOMJAVA-14").message( "Could not find table metadata for involved table \"{{tableName|uq}}\". All involved tables: {{involvedTables}}") diff --git a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java index c8c885c1..9c25a646 100644 --- a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java @@ -1055,8 +1055,7 @@ private PushdownSqlParser getCustomPushdownSqlParserWithTwoTables() { @Test void testParseSelectWithHaving() { - final String sqlAsJson = - "{" + + final String sqlAsJson = "{" + " \"aggregationType\":\"single_group\"," + " \"from\":{" + " \"name\":\"CUSTOMERS\"," + @@ -1109,14 +1108,15 @@ void testParseSelectWithHaving() { final PushdownSqlParser pushdownSqlParser = getCustomPushdownSqlParserWithTwoTables(); final SqlStatementSelect sqlStatementSelect = (SqlStatementSelect) pushdownSqlParser .parseExpression(jsonObject); - assertThat(sqlStatementSelect.getChildren().size(), equalTo(6)); - assertTrue(sqlStatementSelect.hasHaving()); - assertTrue(sqlStatementSelect.hasLimit()); - assertTrue(sqlStatementSelect.hasFilter()); - assertTrue(sqlStatementSelect.hasOrderBy()); + assertAll( + () -> assertThat(sqlStatementSelect.getChildren().size(), equalTo(6)), + () -> assertTrue(sqlStatementSelect.hasHaving()), + () -> assertFalse(sqlStatementSelect.hasGroupBy()), + () -> assertTrue(sqlStatementSelect.hasLimit()), + () -> assertTrue(sqlStatementSelect.hasFilter()), + () -> assertTrue(sqlStatementSelect.hasOrderBy())); } - @Test void testParseSelectWithoutSelectList() { final String sqlAsJson = "{" // @@ -1304,6 +1304,23 @@ void testParseSelectWithoutSelectListAndInvolvedTablesMetadata() { assertThat(exception.getMessage(), containsString("E-VSCOMJAVA-28")); } + @Test + void testParserCopiesInvolvedTablesMetadata() { + final List tables = new ArrayList<>(); + tables.add(new TableMetadata("CUSTOMERS", "", List.of( + ColumnMetadata.builder().name("ID").adapterNotes("").type(createVarChar(200, UTF8)).build()), "")); + final PushdownSqlParser pushdownSqlParser = PushdownSqlParser.createWithTablesMetadata(tables); + tables.clear(); + final JsonObject jsonObject = createJsonObjectFromString("{" + + " \"type\" : \"table\", " + + " \"name\" : \"CUSTOMERS\" " + + "}"); + final SqlTable sqlTable = (SqlTable) pushdownSqlParser.parseExpression(jsonObject); + assertAll(() -> assertThat(sqlTable.getType(), equalTo(TABLE)), + () -> assertThat(sqlTable.getName(), equalTo("CUSTOMERS")), + () -> assertThat(sqlTable.getMetadata().getName(), equalTo("CUSTOMERS"))); + } + @Test void testParseInvalidExpressionType() { final String sqlAsJson = "{" // @@ -1382,4 +1399,13 @@ void testParseSelectWithSingleGroupAggregationWithAggregateFunction() { assertFalse(sqlStatementSelect.hasGroupBy()); assertThat(sqlStatementSelect.getChildren().size(), equalTo(2)); } + + @Test + void testHasAggregateFunctionIgnoresNullValues() { + final SqlFunctionAggregate aggregate = new SqlFunctionAggregate(AggregateFunction.SUM, + List.of(new SqlLiteralExactnumeric(BigDecimal.ONE)), false); + assertAll( + () -> assertThat(PushdownSqlParser.hasAggregateFunction(Collections.singletonList(null)), equalTo(false)), + () -> assertThat(PushdownSqlParser.hasAggregateFunction(Arrays.asList(null, aggregate)), equalTo(true))); + } } From 20878df27bb502231ebbfa5ed5c2a27a7f973720 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Tue, 2 Jun 2026 08:15:57 +0200 Subject: [PATCH 2/6] #316: Improve error handling for invalid log level --- doc/changes/changes_18.0.2.md | 1 + error_code_config.yml | 2 +- .../adapter/request/LoggingConfiguration.java | 24 ++++++++++++++----- .../request/LoggingConfigurationTest.java | 11 +++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/doc/changes/changes_18.0.2.md b/doc/changes/changes_18.0.2.md index 00a63aaa..82f9396f 100644 --- a/doc/changes/changes_18.0.2.md +++ b/doc/changes/changes_18.0.2.md @@ -23,6 +23,7 @@ Code name: Improve code quality * #308: Fixed `SqlLimit` so `limit` and `offset` stay non-negative for the full object lifetime by making the node immutable and aligning the validation message with the accepted zero values. * #307: Fixed `PushdownSqlRenderer` so `HASHTYPE` data types include `bytesize` in rendered pushdown SQL JSON. * #312: Fixed logging utility resource and handler lifecycle issues by closing version metadata streams, falling back when the thread context class loader is missing, defaulting absent version properties to `UNKNOWN`, closing replaced root log handlers, and resetting closed remote socket handlers before reuse. +* #316: Fixed `LoggingConfiguration` so invalid `LOG_LEVEL` values now fail with a structured error message that names the invalid value and lists the available log levels. ## Dependency Updates diff --git a/error_code_config.yml b/error_code_config.yml index 3b299927..d3fe2f4b 100644 --- a/error_code_config.yml +++ b/error_code_config.yml @@ -2,4 +2,4 @@ error-tags: VSCOMJAVA: packages: - com.exasol - highest-index: 45 + highest-index: 46 diff --git a/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java b/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java index bf33a30d..725d781f 100644 --- a/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java +++ b/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java @@ -4,9 +4,12 @@ import static com.exasol.adapter.AdapterProperties.LOG_LEVEL_PROPERTY; import java.io.Serializable; +import java.util.List; import java.util.Map; import java.util.logging.Level; +import com.exasol.errorreporting.ExaError; + /** * This class represents the logging configuration set in the request properties */ @@ -14,6 +17,8 @@ public final class LoggingConfiguration implements Serializable { private static final long serialVersionUID = 1930189191497837644L; private static final int DEFAULT_REMOTE_LOGGING_PORT = 3000; private static final Level DEFAULT_LOG_LEVEL = Level.INFO; + private static final List AVAILABLE_LOG_LEVELS = List.of(Level.OFF, Level.SEVERE, Level.WARNING, Level.INFO, Level.CONFIG, Level.FINE, Level.FINER, + Level.FINEST, Level.ALL); /** {@code true} if the adapter should send its log messages to a remote log receiver */ private final boolean logRemotely; /** Name host name where the log receiver listens */ @@ -105,13 +110,20 @@ public static LoggingConfiguration parseFromPropertiesWithDebugAddressGiven(fina } private static Level parseLogLevel(final Map properties) { - final Level level; - if (properties.containsKey(LOG_LEVEL_PROPERTY)) { - level = Level.parse(properties.get(LOG_LEVEL_PROPERTY)); - } else { - level = DEFAULT_LOG_LEVEL; + if (!properties.containsKey(LOG_LEVEL_PROPERTY)) { + return DEFAULT_LOG_LEVEL; + } + final String configuredLogLevel = properties.get(LOG_LEVEL_PROPERTY); + try { + return Level.parse(configuredLogLevel); + } catch (final IllegalArgumentException exception) { + throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-46") + .message("Invalid value {{log_level_value}} for property {{log_level_property}}.") + .mitigation("Available log levels are: {{available_log_levels}}.") + .parameter("log_level_value", configuredLogLevel) + .parameter("log_level_property", LOG_LEVEL_PROPERTY) + .parameter("available_log_levels", AVAILABLE_LOG_LEVELS).toString(), exception); } - return level; } /** diff --git a/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java b/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java index b4165f3e..2342b8e3 100644 --- a/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java +++ b/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java @@ -5,6 +5,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.util.HashMap; @@ -62,6 +63,16 @@ void testGetLogLevel() { assertThat(createLoggingConfiguration(this.properties).getLogLevel(), equalTo(Level.FINEST)); } + @Test + void testInvalidLogLevelThrowsHelpfulException() { + this.properties.put(LOG_LEVEL_PROPERTY, "invalid"); + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> createLoggingConfiguration(this.properties)); + assertThat(exception.getMessage(), equalTo( + "E-VSCOMJAVA-46: Invalid value 'invalid' for property 'LOG_LEVEL'. " + + "Available log levels are: [OFF, SEVERE, WARNING, INFO, CONFIG, FINE, FINER, FINEST, ALL].")); + } + @Test void testFallBackToLocalLoggingInCaseOfIllegalLoggingPort() { this.properties.put(DEBUG_ADDRESS_PROPERTY, "www.example.org:illegal_non_numeric_port"); From 6b95f07d57c5898f59a4e93999d075498516d389 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Tue, 2 Jun 2026 10:22:07 +0200 Subject: [PATCH 3/6] Replace list involvedTablesMetadata with computed involvedTablesMetadataMap --- .../request/parser/PushdownSqlParser.java | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java index dbe65d6d..0e61fff2 100644 --- a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java @@ -1,6 +1,7 @@ package com.exasol.adapter.request.parser; import static com.exasol.adapter.request.RequestJsonKeys.*; +import static java.util.Collections.unmodifiableMap; import java.math.BigDecimal; import java.util.*; @@ -22,11 +23,18 @@ * Parser for the JSON query AST. */ public final class PushdownSqlParser extends AbstractRequestParser { - private final List involvedTablesMetadata; - private Map involvedTablesMetadataMap; + private final Map involvedTablesMetadataMap; private PushdownSqlParser(final List involvedTablesMetadata) { - this.involvedTablesMetadata = new ArrayList<>(involvedTablesMetadata); + involvedTablesMetadataMap = buildInvolvedTablesMetadataMap(involvedTablesMetadata); + } + + private static Map buildInvolvedTablesMetadataMap(final List involvedTablesMetadata) { + final Map map = new HashMap<>(involvedTablesMetadata.size()); + for (final TableMetadata involvedTableMeta : involvedTablesMetadata) { + map.put(involvedTableMeta.getName(), involvedTableMeta); + } + return unmodifiableMap(map); } private static ExaCharset charSetFromString(final String charset) { @@ -283,16 +291,6 @@ public SqlNode parseExpression(final JsonObject expression) { } } - private Map getInvolvedTablesMetadataMap() { - if (this.involvedTablesMetadataMap == null) { - this.involvedTablesMetadataMap = new HashMap<>(this.involvedTablesMetadata.size()); - for (final TableMetadata involvedTableMeta : this.involvedTablesMetadata) { - this.involvedTablesMetadataMap.put(involvedTableMeta.getName(), involvedTableMeta); - } - } - return this.involvedTablesMetadataMap; - } - private SqlColumn createColumn(final int index, final SqlTable table, final ColumnMetadata columnMetadata) { if (table.hasAlias()) { return new SqlColumn(index, columnMetadata, table.getName(), table.getAlias()); @@ -403,12 +401,11 @@ private SqlNode parseLiteralInterval(final JsonObject expression) { @SuppressWarnings("java:S1192") // tableName is duplicated but that's ok since it's a parameter private List collectAllInvolvedColumns(final SqlNode from) { final List involvedTables = collectInvolvedTables(from); - final Map tableMetadataMap = getInvolvedTablesMetadataMap(); final List selectListElements = new ArrayList<>(); for (final SqlTable table : involvedTables) { final String tableName = table.getName(); - if (tableMetadataMap.containsKey(tableName)) { - final List columns = tableMetadataMap.get(tableName).getColumns(); + if (involvedTablesMetadataMap.containsKey(tableName)) { + final List columns = involvedTablesMetadataMap.get(tableName).getColumns(); for (int i = 0, columnsSize = columns.size(); i < columnsSize; ++i) { selectListElements.add(createColumn(i, table, columns.get(i))); } @@ -732,14 +729,14 @@ private static SqlNodeType fromTypeName(final String typeName) { } private TableMetadata findInvolvedTableMetadata(final String tableName) { - assert this.involvedTablesMetadata != null; - final TableMetadata tableMetadata = getInvolvedTablesMetadataMap().get(tableName); + final TableMetadata tableMetadata = involvedTablesMetadataMap.get(tableName); if (tableMetadata != null) { return tableMetadata; } throw new IllegalStateException(ExaError.messageBuilder("E-VSCOMJAVA-14").message( "Could not find table metadata for involved table \"{{tableName|uq}}\". All involved tables: {{involvedTables}}") - .parameter("tableName", tableName).parameter("involvedTables", this.involvedTablesMetadata.toString()) + .parameter("tableName", tableName) + .parameter("involvedTables", this.involvedTablesMetadataMap.toString()) .toString()); } @@ -753,8 +750,10 @@ private ColumnMetadata findColumnMetadata(final String tableName, final String c throw new IllegalStateException(ExaError.messageBuilder("E-VSCOMJAVA-15").message( "Could not find column metadata for involved table \"{{tableName|uq}}\" and column \"{{columnName|uq}}\". " + "All involved tables: {{involvedTables}}.") - .parameter("tableName", tableName).parameter("columnName", columnName) - .parameter("involvedTables", this.involvedTablesMetadata.toString()).toString()); + .parameter("tableName", tableName) + .parameter("columnName", columnName) + .parameter("involvedTables", this.involvedTablesMetadataMap.toString()) + .toString()); } /** From 1e9061a5639028ab0b9705cda3f459084228346b Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Tue, 2 Jun 2026 10:30:27 +0200 Subject: [PATCH 4/6] Implement review findings --- .../exasol/adapter/request/parser/PushdownSqlParser.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java index 0e61fff2..cbfb0c2f 100644 --- a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java @@ -30,7 +30,7 @@ private PushdownSqlParser(final List involvedTablesMetadata) { } private static Map buildInvolvedTablesMetadataMap(final List involvedTablesMetadata) { - final Map map = new HashMap<>(involvedTablesMetadata.size()); + final Map map = new HashMap<>((int) (involvedTablesMetadata.size() / 0.75f) + 1); for (final TableMetadata involvedTableMeta : involvedTablesMetadata) { map.put(involvedTableMeta.getName(), involvedTableMeta); } @@ -159,14 +159,14 @@ static boolean hasAggregateFunction(final List nodesList) { } while (!expressions.isEmpty()) { final SqlNode expression = expressions.pop(); + if (expression.getType().equals(SqlNodeType.FUNCTION_AGGREGATE)) { + return true; + } for (final SqlNode child : expression.getChildren()) { if (child != null) { expressions.push(child); } } - if (expression.getType().equals(SqlNodeType.FUNCTION_AGGREGATE)) { - return true; - } } return false; } From 31126de154ccef95705abc59a73f55e7a7f20acb Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Tue, 2 Jun 2026 10:35:18 +0200 Subject: [PATCH 5/6] Fix duplicate erorr code --- error_code_config.yml | 2 +- .../java/com/exasol/adapter/request/LoggingConfiguration.java | 2 +- .../com/exasol/adapter/request/LoggingConfigurationTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/error_code_config.yml b/error_code_config.yml index d3fe2f4b..9018a22f 100644 --- a/error_code_config.yml +++ b/error_code_config.yml @@ -2,4 +2,4 @@ error-tags: VSCOMJAVA: packages: - com.exasol - highest-index: 46 + highest-index: 47 diff --git a/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java b/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java index 725d781f..4b2aacbb 100644 --- a/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java +++ b/src/main/java/com/exasol/adapter/request/LoggingConfiguration.java @@ -117,7 +117,7 @@ private static Level parseLogLevel(final Map properties) { try { return Level.parse(configuredLogLevel); } catch (final IllegalArgumentException exception) { - throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-46") + throw new IllegalArgumentException(ExaError.messageBuilder("E-VSCOMJAVA-47") .message("Invalid value {{log_level_value}} for property {{log_level_property}}.") .mitigation("Available log levels are: {{available_log_levels}}.") .parameter("log_level_value", configuredLogLevel) diff --git a/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java b/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java index 2342b8e3..b1daf9d8 100644 --- a/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java +++ b/src/test/java/com/exasol/adapter/request/LoggingConfigurationTest.java @@ -69,7 +69,7 @@ void testInvalidLogLevelThrowsHelpfulException() { final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> createLoggingConfiguration(this.properties)); assertThat(exception.getMessage(), equalTo( - "E-VSCOMJAVA-46: Invalid value 'invalid' for property 'LOG_LEVEL'. " + "E-VSCOMJAVA-47: Invalid value 'invalid' for property 'LOG_LEVEL'. " + "Available log levels are: [OFF, SEVERE, WARNING, INFO, CONFIG, FINE, FINER, FINEST, ALL].")); } From 2f86f96964918f428d42badce87957396a97d386 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl Date: Tue, 2 Jun 2026 11:21:55 +0200 Subject: [PATCH 6/6] Improve test coverage --- .../request/parser/PushdownSqlParser.java | 20 +-- .../request/parser/PushDownSqlParserTest.java | 124 ++++++++++++++++++ 2 files changed, 134 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java index cbfb0c2f..e10bc12b 100644 --- a/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/PushdownSqlParser.java @@ -37,7 +37,7 @@ private static Map buildInvolvedTablesMetadataMap(final L return unmodifiableMap(map); } - private static ExaCharset charSetFromString(final String charset) { + static ExaCharset charSetFromString(final String charset) { if (charset.equals("UTF8")) { return ExaCharset.UTF8; } else if (charset.equals("ASCII")) { @@ -190,7 +190,7 @@ private SqlSelectList parseSelectList(final JsonArray selectList) { } } - private static IntervalType intervalTypeFromString(final String intervalType) { + static IntervalType intervalTypeFromString(final String intervalType) { if (intervalType.equals("DAY TO SECONDS")) { return IntervalType.DAY_TO_SECOND; } else if (intervalType.equals("YEAR TO MONTH")) { @@ -399,7 +399,7 @@ private SqlNode parseLiteralInterval(final JsonObject expression) { } @SuppressWarnings("java:S1192") // tableName is duplicated but that's ok since it's a parameter - private List collectAllInvolvedColumns(final SqlNode from) { + List collectAllInvolvedColumns(final SqlNode from) { final List involvedTables = collectInvolvedTables(from); final List selectListElements = new ArrayList<>(); for (final SqlTable table : involvedTables) { @@ -418,29 +418,29 @@ private List collectAllInvolvedColumns(final SqlNode from) { return selectListElements; } - private DataType getHashtype(final JsonObject dataType) { + private static DataType getHashtype(final JsonObject dataType) { final int byteSize = dataType.getInt(BYTE_SIZE); return DataType.createHashtype(byteSize); } - private DataType getVarchar(final JsonObject dataType) { + private static DataType getVarchar(final JsonObject dataType) { final String charSet = dataType.getString(CHARACTER_SET, "UTF8"); return DataType.createVarChar(dataType.getInt(SIZE), charSetFromString(charSet)); } - private DataType getChar(final JsonObject dataType) { + private static DataType getChar(final JsonObject dataType) { final String charSet = dataType.getString(CHARACTER_SET, "UTF8"); return DataType.createChar(dataType.getInt(SIZE), charSetFromString(charSet)); } - private DataType getTimestamp(final JsonObject dataType) { + private static DataType getTimestamp(final JsonObject dataType) { final boolean withLocalTimezone = dataType.getBoolean(WITH_LOCAL_TIME_ZONE, false); final int precision = dataType.getInt(SchemaMetadataJsonConverter.TIMESTAMP_PRECISION_KEY, DataTypeParser.DEFAULT_TIMESTAMP_PRECISION); return DataType.createTimestamp(withLocalTimezone, precision); } - private DataType getInterval(final JsonObject dataType) { + private static DataType getInterval(final JsonObject dataType) { final int precision = dataType.getInt(PRECISION, 2); final IntervalType intervalType = intervalTypeFromString(dataType.getString(FROM_TO)); if (intervalType == IntervalType.DAY_TO_SECOND) { @@ -451,7 +451,7 @@ private DataType getInterval(final JsonObject dataType) { } } - private DataType getGeometry(final JsonObject dataType) { + private static DataType getGeometry(final JsonObject dataType) { final int srid = dataType.getInt(SRID); return DataType.createGeometry(srid); } @@ -483,7 +483,7 @@ private List collectInvolvedTables(final SqlNode from) { return involvedTables; } - private DataType getDataType(final JsonObject dataType) { + static DataType getDataType(final JsonObject dataType) { final String typeName = dataType.getString(TYPE).toUpperCase(Locale.ROOT); switch (typeName) { case "DECIMAL": diff --git a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java index b1aa2283..8f7e9de0 100644 --- a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java @@ -8,6 +8,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.IOException; import java.io.StringReader; @@ -437,6 +439,34 @@ void testParsePredicateLike() { () -> assertThat(pattern.getValue(), equalTo("a_d"))); } + @Test + void testParsePredicateLikeWithEscapeChar() { + final String sqlAsJson = "{" + + " \"type\" : \"predicate_like\", " + + " \"expression\" : { " + + " \"type\" : \"literal_string\", " + + " \"value\" : \"abcd\" " + + " }, " + + " \"pattern\" : { " + + " \"type\" : \"literal_string\", " + + " \"value\" : \"a!_d\" " + + " }, " + + " \"escapeChar\" : { " + + " \"type\" : \"literal_string\", " + + " \"value\" : \"!\" " + + " } " + + "}"; + final JsonObject jsonObject = createJsonObjectFromString(sqlAsJson); + final SqlPredicateLike sqlPredicateLike = (SqlPredicateLike) this.defaultParser.parseExpression(jsonObject); + final SqlLiteralString left = (SqlLiteralString) sqlPredicateLike.getLeft(); + final SqlLiteralString pattern = (SqlLiteralString) sqlPredicateLike.getPattern(); + final SqlLiteralString escapeChar = (SqlLiteralString) sqlPredicateLike.getEscapeChar(); + assertAll(() -> assertThat(sqlPredicateLike.getType(), equalTo(PREDICATE_LIKE)), + () -> assertThat(left.getValue(), equalTo("abcd")), // + () -> assertThat(pattern.getValue(), equalTo("a!_d")), // + () -> assertThat(escapeChar.getValue(), equalTo("!"))); + } + @Test void testParsePredicateLikeRegexp() { final String sqlAsJson = "{" // @@ -633,6 +663,61 @@ void testParseFunctionScalarCastToTimestamp() { assertThat(sqlFunctionScalarCast.getDataType().toString(), equalTo("TIMESTAMP(5)")); } + @Test + void testGetDataTypeDecimal() { + assertThat(getDataType("{" + + " \"type\" : \"DECIMAL\", " + + " \"precision\" : 31, " + + " \"scale\" : 41 " + + "}"), equalTo(createDecimal(31, 41))); + } + + @Test + void testGetDataTypeDouble() { + assertThat(getDataType("{" + + " \"type\" : \"DOUBLE\" " + + "}"), equalTo(DataType.createDouble())); + } + + @Test + void testGetDataTypeChar() { + assertThat(getDataType("{" + + " \"type\" : \"CHAR\", " + + " \"size\" : 22 " + + "}"), equalTo(DataType.createChar(22, UTF8))); + } + + @Test + void testGetDataTypeBoolean() { + assertThat(getDataType("{" + + " \"type\" : \"BOOLEAN\" " + + "}"), equalTo(DataType.createBool())); + } + + @Test + void testGetDataTypeDate() { + assertThat(getDataType("{" + + " \"type\" : \"DATE\" " + + "}"), equalTo(DataType.createDate())); + } + + @Test + void testGetDataTypeGeometry() { + assertThat(getDataType("{" + + " \"type\" : \"GEOMETRY\", " + + " \"srid\" : 42 " + + "}"), equalTo(DataType.createGeometry(42))); + } + + @Test + void testGetDataTypeUnsupportedType() { + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> getDataType("{" + + " \"type\" : \"UNSUPPORTED\" " + + "}")); + assertThat(exception.getMessage(), equalTo("E-VSCOMJAVA-11: Unsupported data type encountered: 'UNSUPPORTED'.")); + } + @Test void testParseFunctionScalarCast() { final String sqlAsJson = "{" // @@ -1351,6 +1436,17 @@ void testParseSelectWithoutSelectListAndInvolvedTablesMetadata() { assertThat(exception.getMessage(), containsString("E-VSCOMJAVA-28")); } + @Test + void testCollectAllInvolvedColumnsWithMissingMetadata() { + final PushdownSqlParser parser = PushdownSqlParser.createWithTablesMetadata(Collections.emptyList()); + final SqlTable table = mock(SqlTable.class); + when(table.getType()).thenReturn(TABLE); + when(table.getName()).thenReturn("MISSING_TABLE"); + final IllegalStateException exception = assertThrows(IllegalStateException.class, + () -> parser.collectAllInvolvedColumns(table)); + assertThat(exception.getMessage(), equalTo("E-VSCOMJAVA-9: Unable to find metadata for table \"MISSING_TABLE\" during collecting involved columns.")); + } + @Test void testParserCopiesInvolvedTablesMetadata() { final List tables = new ArrayList<>(); @@ -1455,4 +1551,32 @@ void testHasAggregateFunctionIgnoresNullValues() { () -> assertThat(PushdownSqlParser.hasAggregateFunction(Collections.singletonList(null)), equalTo(false)), () -> assertThat(PushdownSqlParser.hasAggregateFunction(Arrays.asList(null, aggregate)), equalTo(true))); } + + @Test + void testCharSetFromStringAscii() { + assertThat(PushdownSqlParser.charSetFromString("ASCII"), equalTo(DataType.ExaCharset.ASCII)); + } + + @Test + void testCharSetFromStringUnsupportedCharset() { + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> PushdownSqlParser.charSetFromString("EBCDIC")); + assertThat(exception.getMessage(), equalTo("E-VSCOMJAVA-12: Unsupported charset encountered: 'EBCDIC'. Supported charsets are 'UTF8' and 'ASCII'.")); + } + + @Test + void testIntervalTypeFromStringYearToMonth() { + assertThat(PushdownSqlParser.intervalTypeFromString("YEAR TO MONTH"), equalTo(DataType.IntervalType.YEAR_TO_MONTH)); + } + + @Test + void testIntervalTypeFromStringUnsupportedInterval() { + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> PushdownSqlParser.intervalTypeFromString("HOUR")); + assertThat(exception.getMessage(), + equalTo("E-VSCOMJAVA-13: Unsupported interval data type encountered: 'HOUR'. Supported intervals are 'DAY TO SECONDS' and 'YEAR TO MONTH'.")); + } + + private DataType getDataType(final String dataTypeJson) { + return PushdownSqlParser.getDataType(createJsonObjectFromString(dataTypeJson)); + } }