Skip to content

Commit

Permalink
fixed review findings incl. implementation for issue #138
Browse files Browse the repository at this point in the history
  • Loading branch information
ckunki committed Mar 10, 2023
1 parent 3ee6a69 commit 573090a
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 13 deletions.
9 changes: 4 additions & 5 deletions doc/changes/changes_10.3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ Code name: Escape Wildcards

## Summary

This release fixes lookup for tables with wildcards underscore `_` and percent `%`.
This release fixes ambiguous results by escaping SQL wildcards such as underscore `_` and percent `%` in names of catalogs, schemas, and tables when retrieving column metadata from JDBC driver.

Even when putting double quotes around the table name older releases of VSCJDBC passed the name to the JDBC driver and the JDBC driver returned the metadata for all matching tables expanding potential wildcards.

The current release fixes this by escaping the wildcards before passing the table name to the JDBC driver.
The release also adds a constructor enabling derived SQL dialects to add additional validators for adapter properties, hence removing the need to override method `AbstractSqlDialect.validateProperties()`.

## Bugfixes

* #136: Column lookup for tables is not escaping wildcards
* #136: Fixed column lookup for tables is not escaping wildcards
* #138: Enabled SQL dialects to add property validators

## Dependency Updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ public abstract class AbstractSqlDialect implements SqlDialect {
*/
protected AbstractSqlDialect(final ConnectionFactory connectionFactory, final AdapterProperties properties,
final Set<String> dialectSpecificProperties) {
this(connectionFactory, properties, dialectSpecificProperties, List.of());
}

/**
* Create a new instance of an {@link AbstractSqlDialect}.
*
* @param connectionFactory factory for JDBC connection to remote data source
* @param properties user properties
* @param dialectSpecificProperties a set of properties that dialect supports additionally to the common set
* {@link com.exasol.adapter.dialects.AbstractSqlDialect#COMMON_SUPPORTED_PROPERTIES}
* @param dialectSpecificPropertyValidators a collection of property validators the dialect wants to apply
* additionally to the common validators
*/
protected AbstractSqlDialect(final ConnectionFactory connectionFactory, final AdapterProperties properties,
final Set<String> dialectSpecificProperties,
final Collection<PropertyValidator> dialectSpecificPropertyValidators) {
this.connectionFactory = connectionFactory;
this.properties = properties;
this.supportedProperties = new SupportedPropertiesValidator() //
Expand All @@ -59,7 +75,8 @@ protected AbstractSqlDialect(final ConnectionFactory connectionFactory, final Ad
.add(PropertyValidator.forStructureElement(supportsJdbcSchemas(), "schemas", SCHEMA_NAME_PROPERTY))
.add(ExceptionHandlingProperty.validator()) //
.add(DataTypeDetection.getValidator()) //
.add(TableCountLimit.getValidator());
.add(TableCountLimit.getValidator()) //
.addAll(dialectSpecificPropertyValidators);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ public List<ColumnMetadata> mapColumns(final String tableName) {
*/
protected List<ColumnMetadata> mapColumns(final String catalogName, final String schemaName,
final String tableName) {
final String tableNameEscaped = Wildcards.escape(tableName);
final String catalogNameEscaped = catalogName == null ? null : Wildcards.escape(catalogName);
final String schemaNameEscaped = schemaName == null ? null : Wildcards.escape(schemaName);
try (final ResultSet remoteColumns = this.connection.getMetaData().getColumns(catalogNameEscaped,
schemaNameEscaped, tableNameEscaped, ANY_COLUMN)) {
try (final ResultSet remoteColumns = getColumnMetadata(catalogName, schemaName, tableName)) {
return getColumnsFromResultSet(remoteColumns);
} catch (final SQLException exception) {
throw new RemoteMetadataReaderException(ExaError.messageBuilder("E-VSCJDBC-1").message(
Expand All @@ -96,6 +92,15 @@ protected List<ColumnMetadata> mapColumns(final String catalogName, final String
}
}

private ResultSet getColumnMetadata(final String catalogName, final String schemaName, final String tableName)
throws SQLException {
return this.connection.getMetaData().getColumns( //
catalogName == null ? null : Wildcards.escape(catalogName), //
schemaName == null ? null : Wildcards.escape(schemaName), //
Wildcards.escape(tableName), //
ANY_COLUMN);
}

/**
* Read the columns result set.
*
Expand Down
14 changes: 12 additions & 2 deletions src/main/java/com/exasol/adapter/properties/ValidatorChain.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.exasol.adapter.properties;

import java.util.ArrayList;
import java.util.List;
import java.util.*;

import com.exasol.adapter.AdapterProperties;

Expand All @@ -23,6 +22,17 @@ public ValidatorChain add(final PropertyValidator validator) {
return this;
}

/**
* Add a list of property validators to the validator chain.
*
* @param validators
* @return this for fluent programming
*/
public ValidatorChain addAll(final Collection<PropertyValidator> validators) {
this.propertyValidators.addAll(validators);
return this;
}

@Override
public void validate(final AdapterProperties properties) throws PropertyValidationException {
for (final PropertyValidator validator : this.propertyValidators) {
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/exasol/adapter/jdbc/WildcardsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class WildcardsTest {
"a _ a, a \\_ a", //
"a % a, a \\% a", //
"a _ b _ c, a \\_ b \\_ c", //
"a_b%c, a\\_b\\%c", //
})
void test(final String input, final String expected) {
assertThat(Wildcards.escape(input), equalTo(expected));
Expand Down

0 comments on commit 573090a

Please sign in to comment.