From f0c8d2365394ba3d4094ebb2be5cf13dd64f4566 Mon Sep 17 00:00:00 2001 From: Marcelo Chiaradia Date: Fri, 12 Feb 2021 12:37:42 +0100 Subject: [PATCH] \#3: Unified error messages with 'error-reporting-java' --- README.md | 2 + doc/changes/changelog.md | 1 + doc/changes/changes_2.0.0.md | 17 +++ errorCodeConfig.yml | 5 + pom.xml | 18 ++- .../oracle/OracleColumnMetadataReader.java | 6 +- .../OracleConnectionDefinitionBuilder.java | 9 +- .../dialects/oracle/OracleIdentifier.java | 10 +- .../dialects/oracle/OracleQueryRewriter.java | 10 +- .../dialects/oracle/OracleSqlDialect.java | 15 ++- .../oracle/OracleSqlDialectFactory.java | 10 +- .../oracle/OracleSqlGenerationVisitor.java | 58 ++------- .../OracleColumnMetadataReaderTest.java | 19 +-- ...OracleConnectionDefinitionBuilderTest.java | 6 +- .../dialects/oracle/OracleIdentifierTest.java | 5 +- .../oracle/OracleQueryRewriterTest.java | 10 +- .../dialects/oracle/OracleSqlDialectTest.java | 27 +++- .../OracleSqlGenerationVisitorTest.java | 119 +++--------------- .../oracle/OracleTableMetadataReaderTest.java | 6 +- 19 files changed, 136 insertions(+), 217 deletions(-) create mode 100644 doc/changes/changes_2.0.0.md create mode 100644 errorCodeConfig.yml diff --git a/README.md b/README.md index e66941b..b6040a1 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,7 @@ Running the Virtual Schema requires a Java Runtime version 11 or later. | [Maven Dependency Plugin][maven-dependency-plugin] | Accessing to test dependencies | Apache License 2.0 | | [Artifact Reference Checker Plugin][artifact-ref-checker-plugin] | Check if artifact is referenced with correct version | MIT License | | [Project Keeper Maven Plugin][project-keeper-maven-plugin] | Checking project structure | MIT License | +| [Error Code Crawler Plugin][error-code-crawler-plugin] | Analyzing used error messages | MIT License | | [Sonatype OSS Index Maven Plugin][sonatype-oss-index-maven-plugin] | Checking dependencies vulnerability | ASL2 | [virtual-schema-common-jdbc]: https://github.com/exasol/virtual-schema-common-jdbc @@ -101,6 +102,7 @@ Running the Virtual Schema requires a Java Runtime version 11 or later. [artifact-ref-checker-plugin]: https://github.com/exasol/artifact-reference-checker-maven-plugin [maven-dependency-plugin]: https://maven.apache.org/plugins/maven-dependency-plugin/ [project-keeper-maven-plugin]: https://github.com/exasol/project-keeper-maven-plugin +[error-code-crawler-plugin]: https://github.com/exasol/error-code-crawler-maven-plugin [sonatype-oss-index-maven-plugin]: https://sonatype.github.io/ossindex-maven/maven-plugin/ [virtual-schemas-user-guide]: https://docs.exasol.com/database_concepts/virtual_schemas.htm diff --git a/doc/changes/changelog.md b/doc/changes/changelog.md index a061455..e111020 100644 --- a/doc/changes/changelog.md +++ b/doc/changes/changelog.md @@ -1,3 +1,4 @@ # Changes +* [2.0.0](changes_2.0.0.md) * [1.0.0](changes_1.0.0.md) \ No newline at end of file diff --git a/doc/changes/changes_2.0.0.md b/doc/changes/changes_2.0.0.md new file mode 100644 index 0000000..b0c5f9e --- /dev/null +++ b/doc/changes/changes_2.0.0.md @@ -0,0 +1,17 @@ +# Oracle Virtual Schemas 2.0.0, released 2021-??-?? + +Code name: + +## Summary + +The `SQL_DIALECT` property used when executing a `CREATE VIRTUAL SCHEMA` from the Exasol database is obsolete from this version. Please, do not provide this property anymore. + +## Features / Enhancements + +* 3: Unified error messages with `error-reporting-java` + +## Plugin Dependencies + +* Added `com.exasol:error-code-crawler-maven-plugin:0.1.1` +* Updated `com.exasol:error-reporting-java:0.2.0` to `0.2.2` +* Updated `com.exasol:virtual-schema-common-jdbc:8.0.0` to `9.0.1` \ No newline at end of file diff --git a/errorCodeConfig.yml b/errorCodeConfig.yml new file mode 100644 index 0000000..7952b4e --- /dev/null +++ b/errorCodeConfig.yml @@ -0,0 +1,5 @@ +error-tags: + VS-ORA: + packages: + - com.exasol.adapter.dialects.oracle + highest-index: 1 \ No newline at end of file diff --git a/pom.xml b/pom.xml index 4667132..b4b3398 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,9 @@ UTF-8 11 3.0.0-M3 - 8.0.0 + 9.0.1 + 0.2.2 + 0.1.1 1.15.0 target/site/jacoco/jacoco.xml,target/site/jacoco-it/jacoco.xml @@ -54,7 +56,7 @@ com.exasol error-reporting-java - 0.2.0 + ${error.reporting.java.version} @@ -294,6 +296,18 @@ + + com.exasol + error-code-crawler-maven-plugin + ${error.code.crawler.version} + + + + verify + + + + org.sonatype.ossindex.maven ossindex-maven-plugin diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReader.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReader.java index c4c0981..8df482f 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReader.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReader.java @@ -8,7 +8,7 @@ import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.dialects.IdentifierConverter; import com.exasol.adapter.jdbc.BaseColumnMetadataReader; -import com.exasol.adapter.jdbc.JdbcTypeDescription; +import com.exasol.adapter.jdbc.JDBCTypeDescription; import com.exasol.adapter.metadata.DataType; /** @@ -36,7 +36,7 @@ public OracleColumnMetadataReader(final Connection connection, final AdapterProp } @Override - public DataType mapJdbcType(final JdbcTypeDescription jdbcTypeDescription) { + public DataType mapJdbcType(final JDBCTypeDescription jdbcTypeDescription) { switch (jdbcTypeDescription.getJdbcType()) { case Types.DECIMAL: case Types.NUMERIC: @@ -54,7 +54,7 @@ public DataType mapJdbcType(final JdbcTypeDescription jdbcTypeDescription) { } } - protected DataType mapNumericType(final JdbcTypeDescription jdbcTypeDescription) { + protected DataType mapNumericType(final JDBCTypeDescription jdbcTypeDescription) { final int decimalScale = jdbcTypeDescription.getDecimalScale(); if (decimalScale == ORACLE_MAGIC_NUMBER_SCALE) { return workAroundNumberWithoutScaleAndPrecision(); diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilder.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilder.java index 1aaf06d..411e526 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilder.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilder.java @@ -6,6 +6,7 @@ import com.exasol.ExaConnectionInformation; import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.jdbc.BaseConnectionDefinitionBuilder; +import com.exasol.errorreporting.ExaError; /** * This class implements an Oracle-specific connection definition builder. @@ -25,9 +26,11 @@ private String buildImportFromOraConnectionDefinition(final AdapterProperties pr if (properties.containsKey(ORACLE_CONNECTION_NAME_PROPERTY)) { return buildOracleConnectionDefinitionFromOracleConnectionOnly(properties); } else { - throw new IllegalArgumentException("If you enable IMPORT FROM ORA with property \"" + ORACLE_IMPORT_PROPERTY - + "\" you also need to provide the name of an Oracle connection with \"" - + ORACLE_CONNECTION_NAME_PROPERTY + "\"."); + throw new IllegalArgumentException(ExaError.messageBuilder("E-VS-ORA-3") + .message("If you enable IMPORT FROM ORA with property {{OracleImportProperty}} " + + "you also need to provide the name of an Oracle connection with {{OracleConnectionNameProperty}}.") + .parameter("OracleImportProperty", ORACLE_IMPORT_PROPERTY) + .parameter("OracleConnectionNameProperty", ORACLE_CONNECTION_NAME_PROPERTY).toString()); } } diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleIdentifier.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleIdentifier.java index 5d4a78a..e5da9aa 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleIdentifier.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleIdentifier.java @@ -3,6 +3,7 @@ import java.util.Objects; import com.exasol.db.Identifier; +import com.exasol.errorreporting.ExaError; /** * Represents an identifier in the Oracle database. @@ -34,10 +35,11 @@ public static OracleIdentifier of(final String id) { if (validate(id)) { return new OracleIdentifier(id); } else { - throw new AssertionError("E-ID-3: Unable to create identifier \"" + id // - + "\" because it contains illegal characters." // - + " For information about valid identifiers, please refer to" // - + " https://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements008.htm"); + throw new AssertionError(ExaError.messageBuilder("E-VS-ORA-2") + .message("Unable to create identifier {{id}} because it contains illegal characters." // + + " For information about valid identifiers, please refer to" // + + " https://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements008.htm") + .parameter("id", id).toString()); } } diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriter.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriter.java index 567c47d..e706595 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriter.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriter.java @@ -2,9 +2,8 @@ import java.sql.SQLException; -import com.exasol.adapter.dialects.AbstractQueryRewriter; import com.exasol.adapter.dialects.SqlDialect; -import com.exasol.adapter.jdbc.ConnectionDefinitionBuilder; +import com.exasol.adapter.dialects.rewriting.AbstractQueryRewriter; import com.exasol.adapter.jdbc.RemoteMetadataReader; /** @@ -18,12 +17,7 @@ public class OracleQueryRewriter extends AbstractQueryRewriter { * @param remoteMetadataReader reader for metadata from the remote data source */ public OracleQueryRewriter(final SqlDialect dialect, final RemoteMetadataReader remoteMetadataReader) { - super(dialect, remoteMetadataReader); - } - - @Override - protected ConnectionDefinitionBuilder createConnectionDefinitionBuilder() { - return new OracleConnectionDefinitionBuilder(); + super(dialect, remoteMetadataReader, new OracleConnectionDefinitionBuilder()); } @Override diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java index d0d9c2f..6153cd5 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialect.java @@ -16,9 +16,13 @@ import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.capabilities.Capabilities; import com.exasol.adapter.dialects.*; +import com.exasol.adapter.dialects.rewriting.ImportIntoTemporaryTableQueryRewriter; +import com.exasol.adapter.dialects.rewriting.SqlGenerationContext; import com.exasol.adapter.jdbc.*; import com.exasol.adapter.metadata.DataType; -import com.exasol.adapter.sql.*; +import com.exasol.adapter.sql.AggregateFunction; +import com.exasol.adapter.sql.ScalarFunction; +import com.exasol.errorreporting.ExaError; /** * This class implements the Oracle SQL dialect. @@ -109,7 +113,7 @@ private DataType getOracleNumberTypeFromProperty() { } @Override - public SqlNodeVisitor getSqlGenerationVisitor(final SqlGenerationContext context) { + public SqlGenerator getSqlGenerator(final SqlGenerationContext context) { return new OracleSqlGenerationVisitor(this, context); } @@ -160,8 +164,9 @@ protected RemoteMetadataReader createRemoteMetadataReader() { try { return new OracleMetadataReader(this.connectionFactory.getConnection(), this.properties); } catch (final SQLException exception) { - throw new RemoteMetadataReaderException( - "Unable to create Oracle remote metadata reader. Caused by: " + exception.getMessage(), exception); + throw new RemoteMetadataReaderException(ExaError.messageBuilder("E-VS-ORA-1") + .message("Unable to create Oracle remote metadata reader. Caused by: {{cause}}") + .unquotedParameter("cause", exception.getMessage()).toString(), exception); } } @@ -170,7 +175,7 @@ protected QueryRewriter createQueryRewriter() { if (this.isImportFromOraEnabled()) { return new OracleQueryRewriter(this, createRemoteMetadataReader()); } - return new ImportIntoQueryRewriter(this, createRemoteMetadataReader(), this.connectionFactory); + return new ImportIntoTemporaryTableQueryRewriter(this, createRemoteMetadataReader(), this.connectionFactory); } private boolean isImportFromOraEnabled() { diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectFactory.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectFactory.java index d19b390..22e138a 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectFactory.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectFactory.java @@ -19,11 +19,11 @@ public String getSqlDialectName() { public SqlDialect createSqlDialect(final ConnectionFactory connectionFactory, final AdapterProperties properties) { return new OracleSqlDialect(connectionFactory, properties); } - + @Override - public String getSqlDialectVersion() { - final VersionCollector versionCollector = new VersionCollector( - "META-INF/maven/com.exasol/mysql-virtual-schema/pom.properties"); + public String getSqlDialectVersion() { + final VersionCollector versionCollector = new VersionCollector( + "META-INF/maven/com.exasol/oracle-virtual-schema/pom.properties"); return versionCollector.getVersionNumber(); - } + } } \ No newline at end of file diff --git a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitor.java b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitor.java index eccce6e..852934e 100644 --- a/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitor.java +++ b/src/main/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitor.java @@ -7,7 +7,9 @@ import com.exasol.adapter.AdapterException; import com.exasol.adapter.dialects.*; -import com.exasol.adapter.metadata.*; +import com.exasol.adapter.dialects.rewriting.SqlGenerationContext; +import com.exasol.adapter.dialects.rewriting.SqlGenerationVisitor; +import com.exasol.adapter.metadata.DataType; import com.exasol.adapter.sql.*; /** @@ -84,10 +86,8 @@ private String getSqlStatementSelectWithOffset(final SqlStatementSelect select, throws AdapterException { final StringBuilder builder = new StringBuilder(); builder.append("SELECT "); - if (select.getSelectList().isRequestAnyColumn()) { + if (!select.getSelectList().hasExplicitColumnsList()) { return "1"; - } else if (select.getSelectList().isSelectStar()) { - appendSelectStar(select, builder); } else { final int numberOfExpressions = select.getSelectList().getExpressions().size(); builder.append(String.join(", ", buildAliases(numberOfExpressions))); @@ -103,16 +103,6 @@ private String getSqlStatementSelectWithOffset(final SqlStatementSelect select, return builder.toString(); } - private void appendSelectStar(final SqlStatementSelect select, final StringBuilder builder) { - int numberOfColumns = 0; - final List tableMetadata = new ArrayList<>(); - SqlGenerationHelper.addMetadata(select.getFromClause(), tableMetadata); - for (final TableMetadata tableMeta : tableMetadata) { - numberOfColumns += tableMeta.getColumns().size(); - } - builder.append(String.join(", ", buildAliases(numberOfColumns))); - } - private List buildAliases(final int numSelectListElements) { final List aliases = new ArrayList<>(numSelectListElements); for (int i = 0; i < numSelectListElements; i++) { @@ -133,7 +123,7 @@ private String getSqlStatementSelectWithoutOffset(final SqlStatementSelect selec @Override public String visit(final SqlSelectList selectList) throws AdapterException { - if (selectList.isRequestAnyColumn()) { + if (!selectList.hasExplicitColumnsList()) { return "1"; } else { return getSqlSelectList(selectList); @@ -142,12 +132,8 @@ public String visit(final SqlSelectList selectList) throws AdapterException { private String getSqlSelectList(final SqlSelectList selectList) throws AdapterException { final List selectListElements = new ArrayList<>(); - if (selectList.isSelectStar()) { - getSelectStarList(selectList, selectListElements); - } else { - for (final SqlNode node : selectList.getExpressions()) { - selectListElements.add(node.accept(this)); - } + for (final SqlNode node : selectList.getExpressions()) { + selectListElements.add(node.accept(this)); } if (this.requiresSelectListAliasesForLimit) { addColumnAliases(selectListElements); @@ -155,34 +141,6 @@ private String getSqlSelectList(final SqlSelectList selectList) throws AdapterEx return String.join(", ", selectListElements); } - private void getSelectStarList(final SqlSelectList selectList, final List selectListElements) - throws AdapterException { - final SqlStatementSelect select = (SqlStatementSelect) selectList.getParent(); - final boolean selectListRequiresCasts = isSelectListRequiresCasts(selectList, selectListElements, select); - if (!this.requiresSelectListAliasesForLimit && !selectListRequiresCasts) { - selectListElements.clear(); - selectListElements.add("*"); - } - } - - private boolean isSelectListRequiresCasts(final SqlSelectList selectList, final List selectListElements, - final SqlStatementSelect select) throws AdapterException { - boolean selectListRequiresCasts = false; - int columnId = 0; - final List tableMetadata = new ArrayList<>(); - SqlGenerationHelper.addMetadata(select.getFromClause(), tableMetadata); - for (final TableMetadata tableMeta : tableMetadata) { - for (final ColumnMetadata columnMeta : tableMeta.getColumns()) { - final SqlColumn sqlColumn = new SqlColumn(columnId, columnMeta); - sqlColumn.setParent(selectList); - selectListRequiresCasts |= nodeRequiresCast(sqlColumn); - selectListElements.add(sqlColumn.accept(this)); - ++columnId; - } - } - return selectListRequiresCasts; - } - private boolean nodeRequiresCast(final SqlNode node) throws AdapterException { if (node.getType() == SqlNodeType.COLUMN) { return checkIfColumnRequiresCast((SqlColumn) node); @@ -316,7 +274,7 @@ private String getLiteralString(final String literalString, final boolean b, fin @Override public String visit(final SqlLiteralDouble literal) { - final String literalString = Double.toString(literal.getValue()); + final String literalString = literal.getValue(); return getLiteralString(literalString, literal.hasParent(), literal.getParent()); } diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReaderTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReaderTest.java index 7e40fe2..fbd4f92 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReaderTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleColumnMetadataReaderTest.java @@ -5,14 +5,13 @@ import static org.hamcrest.Matchers.equalTo; import java.sql.Types; -import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.dialects.BaseIdentifierConverter; -import com.exasol.adapter.jdbc.JdbcTypeDescription; +import com.exasol.adapter.jdbc.JDBCTypeDescription; import com.exasol.adapter.metadata.DataType; class OracleColumnMetadataReaderTest { @@ -28,16 +27,16 @@ protected OracleColumnMetadataReader createDefaultOracleColumnMetadataReader() { BaseIdentifierConverter.createDefault()); } - private JdbcTypeDescription createTypeDescriptionForNumeric(final int precision, final int scale) { + private JDBCTypeDescription createTypeDescriptionForNumeric(final int precision, final int scale) { final int octetLength = 10; - return new JdbcTypeDescription(Types.NUMERIC, scale, precision, octetLength, "NUMERIC"); + return new JDBCTypeDescription(Types.NUMERIC, scale, precision, octetLength, "NUMERIC"); } @Test void testMapColumnTypeWithMagicScale() { final int precision = 10; final int scale = OracleColumnMetadataReader.ORACLE_MAGIC_NUMBER_SCALE; - final JdbcTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); + final JDBCTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); assertThat(this.columnMetadataReader.mapJdbcType(typeDescription), equalTo(createMaximumSizeVarChar(DataType.ExaCharset.UTF8))); } @@ -46,7 +45,7 @@ void testMapColumnTypeWithMagicScale() { void testMapNumericColumnTypeWithMaximumDecimalPrecision() { final int precision = DataType.MAX_EXASOL_DECIMAL_PRECISION; final int scale = 0; - final JdbcTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); + final JDBCTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); assertThat(this.columnMetadataReader.mapJdbcType(typeDescription), equalTo(DataType.createDecimal(precision, scale))); } @@ -55,14 +54,8 @@ void testMapNumericColumnTypeWithMaximumDecimalPrecision() { void testMapColumnTypeWithZeroPrecision() { final int precision = 0; final int scale = 0; - final JdbcTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); + final JDBCTypeDescription typeDescription = createTypeDescriptionForNumeric(precision, scale); assertThat(this.columnMetadataReader.mapJdbcType(typeDescription), equalTo(DataType.createDecimal(DataType.MAX_EXASOL_DECIMAL_PRECISION, scale))); } - - private OracleColumnMetadataReader createParameterizedColumnMetadataReader( - final Map rawProperties) { - return new OracleColumnMetadataReader(null, new AdapterProperties(rawProperties), - BaseIdentifierConverter.createDefault()); - } } \ No newline at end of file diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilderTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilderTest.java index 61bd9d1..b2224a3 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilderTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleConnectionDefinitionBuilderTest.java @@ -22,7 +22,7 @@ void testBuildConnectionDefinition() { ORACLE_IMPORT_PROPERTY, "true", // ORACLE_CONNECTION_NAME_PROPERTY, "ora_connection", // CONNECTION_NAME_PROPERTY, "jdbc_connection")); - assertThat(connectionDefinitionBuilder.buildConnectionDefinition(properties, null), + assertThat(this.connectionDefinitionBuilder.buildConnectionDefinition(properties, null), containsString("AT ora_connection")); } @@ -32,7 +32,7 @@ void testBuildConnectionDefinitionMissingPropertyException() { ORACLE_IMPORT_PROPERTY, "true", // CONNECTION_NAME_PROPERTY, "ora_connection")); final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> connectionDefinitionBuilder.buildConnectionDefinition(properties, null)); - assertThat(exception.getMessage(), containsString("If you enable IMPORT FROM ORA with property")); + () -> this.connectionDefinitionBuilder.buildConnectionDefinition(properties, null)); + assertThat(exception.getMessage(), containsString("E-VS-ORA-3")); } } \ No newline at end of file diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleIdentifierTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleIdentifierTest.java index 6318d6f..f7dac75 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleIdentifierTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleIdentifierTest.java @@ -1,5 +1,7 @@ package com.exasol.adapter.dialects.oracle; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -19,7 +21,8 @@ void testCreateValidIdentifier(final String identifier) { @ParameterizedTest @ValueSource(strings = { "\"testtable\"", "test\"table" }) void testCreateInvalidIdentifier(final String identifier) { - assertThrows(AssertionError.class, () -> OracleIdentifier.of(identifier)); + final AssertionError assertionError = assertThrows(AssertionError.class, () -> OracleIdentifier.of(identifier)); + assertThat(assertionError.getMessage(), containsString("E-VS-ORA-2")); } @Test diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriterTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriterTest.java index e77b407..7bc9d75 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriterTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleQueryRewriterTest.java @@ -4,7 +4,6 @@ import static com.exasol.adapter.dialects.oracle.OracleProperties.ORACLE_IMPORT_PROPERTY; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; import java.sql.SQLException; import java.util.Map; @@ -18,6 +17,7 @@ import com.exasol.adapter.AdapterException; import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.dialects.*; +import com.exasol.adapter.dialects.rewriting.AbstractQueryRewriterTestBase; import com.exasol.adapter.jdbc.ConnectionFactory; import com.exasol.adapter.sql.TestSqlStatementFactory; @@ -40,12 +40,4 @@ void testRewriteToImportFromOraWithConnectionDetailsInProperties( assertThat(queryRewriter.rewrite(this.statement, EXA_METADATA, properties), equalTo("IMPORT FROM ORA AT ora_connection STATEMENT 'SELECT TO_CHAR(1) FROM \"DUAL\"'")); } - - @Test - void testConnectionDefinitionBuilderClass() { - final SqlDialect dialect = new OracleSqlDialect(null, AdapterProperties.emptyProperties()); - final OracleQueryRewriter queryRewriter = new OracleQueryRewriter(dialect, null); - assertThat(queryRewriter.createConnectionDefinitionBuilder(), - instanceOf(OracleConnectionDefinitionBuilder.class)); - } } \ No newline at end of file diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectTest.java index 2dfeb3d..8d0ff4c 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlDialectTest.java @@ -14,7 +14,9 @@ import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; +import java.sql.SQLException; import java.util.HashMap; import java.util.Map; @@ -31,8 +33,11 @@ import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.capabilities.Capabilities; -import com.exasol.adapter.dialects.*; +import com.exasol.adapter.dialects.PropertyValidationException; +import com.exasol.adapter.dialects.SqlDialect; +import com.exasol.adapter.dialects.rewriting.ImportIntoTemporaryTableQueryRewriter; import com.exasol.adapter.jdbc.ConnectionFactory; +import com.exasol.adapter.jdbc.RemoteMetadataReaderException; @ExtendWith(MockitoExtension.class) class OracleSqlDialectTest { @@ -78,6 +83,19 @@ void testGetCapabilities() { TO_TIMESTAMP, BIT_AND, BIT_TO_NUM, CASE, NULLIFZERO, ZEROIFNULL))); } + @Test + void testMetadataReaderClass() { + assertThat(this.dialect.createRemoteMetadataReader(), instanceOf(OracleMetadataReader.class)); + } + + @Test + void testCreateRemoteMetadataReaderConnectionFails() throws SQLException { + when(this.connectionFactoryMock.getConnection()).thenThrow(new SQLException()); + final RemoteMetadataReaderException exception = assertThrows(RemoteMetadataReaderException.class, + this.dialect::createRemoteMetadataReader); + assertThat(exception.getMessage(), containsString("E-VS-ORA-1")); + } + @CsvSource({ "FALSE, FALSE, JDBC", // "TRUE, FALSE, LOCAL", // "FALSE, TRUE, ORA" }) @@ -92,8 +110,7 @@ void testGetImportTypeLocal(final String local, final String fromOracle, final S @Test void testCheckOracleSpecificPropertyConsistencyInvalidDialect() { final SqlDialect sqlDialect = new OracleSqlDialect(null, - new AdapterProperties(Map.of(SQL_DIALECT_PROPERTY, "ORACLE", // - CONNECTION_NAME_PROPERTY, "MY_CONN", // + new AdapterProperties(Map.of(CONNECTION_NAME_PROPERTY, "MY_CONN", // "ORACLE_CAST_NUMBER_TO_DECIMAL_WITH_PRECISION_AND_SCALE", "MY_CONN"))); assertThrows(PropertyValidationException.class, sqlDialect::validateProperties); } @@ -101,7 +118,6 @@ void testCheckOracleSpecificPropertyConsistencyInvalidDialect() { @Test void testValidateCatalogProperty() { final SqlDialect sqlDialect = new OracleSqlDialect(null, new AdapterProperties(Map.of( // - SQL_DIALECT_PROPERTY, "ORACLE", // CONNECTION_NAME_PROPERTY, "MY_CONN", // CATALOG_NAME_PROPERTY, "MY_CATALOG"))); final PropertyValidationException exception = assertThrows(PropertyValidationException.class, @@ -113,7 +129,6 @@ void testValidateCatalogProperty() { @Test void testValidateSchemaProperty() throws PropertyValidationException { final AdapterProperties adapterProperties = new AdapterProperties(Map.of( // - SQL_DIALECT_PROPERTY, "ORACLE", // CONNECTION_NAME_PROPERTY, "MY_CONN", // SCHEMA_NAME_PROPERTY, "MY_SCHEMA")); final SqlDialect sqlDialect = new OracleSqlDialect(null, adapterProperties); @@ -135,7 +150,7 @@ private AdapterProperties getAdapaterPropertiesWithImportFromOracle() { @Test void testQueryRewriterClassWhitImportInto() { - assertThat(this.dialect.createQueryRewriter(), instanceOf(ImportIntoQueryRewriter.class)); + assertThat(this.dialect.createQueryRewriter(), instanceOf(ImportIntoTemporaryTableQueryRewriter.class)); } @CsvSource({ "tableName, \"tableName\"", // diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitorTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitorTest.java index 3d73b79..00048af 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitorTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleSqlGenerationVisitorTest.java @@ -1,6 +1,5 @@ package com.exasol.adapter.dialects.oracle; -import static com.exasol.adapter.dialects.VisitorAssertions.assertSqlNodeConvertedToAsterisk; import static com.exasol.adapter.dialects.VisitorAssertions.assertSqlNodeConvertedToOne; import static com.exasol.adapter.sql.AggregateFunction.*; import static com.exasol.adapter.sql.ScalarFunction.*; @@ -25,7 +24,8 @@ import com.exasol.adapter.AdapterProperties; import com.exasol.adapter.adapternotes.ColumnAdapterNotes; import com.exasol.adapter.adapternotes.ColumnAdapterNotesJsonConverter; -import com.exasol.adapter.dialects.*; +import com.exasol.adapter.dialects.SqlDialect; +import com.exasol.adapter.dialects.rewriting.SqlGenerationContext; import com.exasol.adapter.metadata.*; import com.exasol.adapter.sql.*; import com.exasol.sql.SqlNormalizer; @@ -78,20 +78,6 @@ void testVisitSqlStatementSelectWithLimitAnyValue() throws AdapterException { assertThat(this.visitor.visit(sqlStatementSelect), equalTo("1")); } - @Test - void testVisitSqlStatementSelectWithLimitSelectStar() throws AdapterException { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); - final TableMetadata tableMetadata = new TableMetadata("", "", Collections.emptyList(), ""); - final SqlTable fromClause = new SqlTable("test_table_name", tableMetadata); - final SqlLimit limit = new SqlLimit(10, 3); - final SqlStatementSelect sqlStatementSelect = SqlStatementSelect.builder().selectList(selectList) - .fromClause(fromClause).limit(limit).build(); - assertThat(this.visitor.visit(sqlStatementSelect), - equalTo("SELECT FROM ( SELECT LIMIT_SUBSELECT.*, ROWNUM " - + "ROWNUM_SUB FROM ( SELECT FROM \"test_schema\".\"test_table_name\" ) LIMIT_SUBSELECT WHERE " - + "ROWNUM <= 13 ) WHERE ROWNUM_SUB > 3")); - } - @Test void testVisitSqlStatementSelectWithLimitRegularSelectList() throws AdapterException { final SqlSelectList selectList = SqlSelectList @@ -127,56 +113,6 @@ void testVisitSqlSelectListRequiresAnyColumn() throws AdapterException { assertSqlNodeConvertedToOne(sqlSelectList, this.visitor); } - @Test - void testVisitSqlSelectListSelectStar() throws AdapterException { - final SqlSelectList selectList = createSqlSelectStarListWithOneColumn( - "{\"jdbcDataType\":16, \"typeName\":\"BOOLEAN\"}", DataType.createBool()); - assertSqlNodeConvertedToAsterisk(selectList, this.visitor); - } - - @CsvSource({ "NUMBER", "INTERVAL", "BINARY_FLOAT", "BINARY_DOUBLE" }) - @ParameterizedTest - void testVisitSqlSelectListSelectStarCastToChar(final String dataType) throws AdapterException { - final SqlSelectList selectList = createSqlSelectStarListWithOneColumn( - "{\"jdbcDataType\":2, \"typeName\":\"" + dataType + "\"}", - DataType.createVarChar(50, DataType.ExaCharset.UTF8)); - assertThat(this.visitor.visit(selectList), equalTo("TO_CHAR(\"test_column\")")); - } - - private SqlSelectList createSqlSelectStarListWithOneColumn(final String adapterNotes, final DataType dataType) { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); - final List columns = new ArrayList<>(); - columns.add(ColumnMetadata.builder().name("test_column").adapterNotes(adapterNotes).type(dataType).build()); - final TableMetadata tableMetadata = new TableMetadata("", "", columns, ""); - final SqlTable fromClause = new SqlTable("", tableMetadata); - final SqlNode sqlStatementSelect = SqlStatementSelect.builder().selectList(selectList).fromClause(fromClause) - .build(); - selectList.setParent(sqlStatementSelect); - return selectList; - } - - @Test - void testVisitSqlSelectListSelectStarWithTimestamp() throws AdapterException { - final SqlSelectList selectList = createSqlSelectStarListWithOneColumn( - "{\"jdbcDataType\":2, \"typeName\":\"TIMESTAMP\"}", - DataType.createVarChar(50, DataType.ExaCharset.UTF8)); - assertThat(this.visitor.visit(selectList), equalTo( - "TO_TIMESTAMP(TO_CHAR(\"test_column\", 'YYYY-MM-DD HH24:MI:SS.FF3'), 'YYYY-MM-DD HH24:MI:SS.FF3')")); - } - - @Test - void testVisitSqlSelectListSelectStarNumberCastToDecimal() throws AdapterException { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); - final List columns = new ArrayList<>(); - columns.add(ColumnMetadata.builder().name("test_column") - .adapterNotes("{\"jdbcDataType\":2, \"typeName\":\"NUMBER\"}").type(DataType.createDouble()).build()); - final TableMetadata tableMetadata = new TableMetadata("", "", columns, ""); - final SqlTable fromClause = new SqlTable("", tableMetadata); - final SqlNode select = SqlStatementSelect.builder().selectList(selectList).fromClause(fromClause).build(); - selectList.setParent(select); - assertThat(this.visitor.visit(selectList), equalTo("CAST(\"test_column\" AS DECIMAL(0,0))")); - } - @Test void testVisitSqlSelectListRegularSelectList() throws AdapterException { final SqlSelectList selectList = SqlSelectList @@ -203,7 +139,7 @@ void testVisitSqlLiteralExactnumeric() { @Test void testVisitSqlLiteralExactnumericInSelectList() { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); + final SqlSelectList selectList = SqlSelectList.createAnyValueSelectList(); final SqlLiteralExactnumeric literalExactnumeric = new SqlLiteralExactnumeric(new BigDecimal("5.9")); literalExactnumeric.setParent(selectList); assertThat(this.visitor.visit(literalExactnumeric), equalTo("TO_CHAR(5.9)")); @@ -212,15 +148,15 @@ void testVisitSqlLiteralExactnumericInSelectList() { @Test void testVisitSqlLiteralDouble() { final SqlLiteralDouble literalDouble = new SqlLiteralDouble(10.6); - assertThat(this.visitor.visit(literalDouble), equalTo("10.6")); + assertThat(this.visitor.visit(literalDouble), equalTo("1.06E1")); } @Test void testVisitSqlLiteralDoubleInSelectList() { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); + final SqlSelectList selectList = SqlSelectList.createAnyValueSelectList(); final SqlLiteralDouble literalDouble = new SqlLiteralDouble(10.6); literalDouble.setParent(selectList); - assertThat(this.visitor.visit(literalDouble), equalTo("TO_CHAR(10.6)")); + assertThat(this.visitor.visit(literalDouble), equalTo("TO_CHAR(1.06E1)")); } @Test @@ -228,7 +164,7 @@ void testVisitSqlFunctionAggregateGroupConcat() throws AdapterException { final SqlFunctionAggregateGroupConcat aggregateGroupConcat = SqlFunctionAggregateGroupConcat .builder(new SqlLiteralDouble(10.5)).separator(new SqlLiteralString("'")).build(); assertThat(this.visitor.visit(aggregateGroupConcat), - equalTo("LISTAGG(10.5, '''') WITHIN GROUP(ORDER BY 10.5)")); + equalTo("LISTAGG(1.05E1, '''') WITHIN GROUP(ORDER BY 1.05E1)")); } @Test @@ -245,7 +181,7 @@ void testVisitSqlFunctionAggregateGroupConcatWithOrderBy() throws AdapterExcepti .builder(new SqlLiteralDouble(10.5)).separator(new SqlLiteralString("'")).orderBy(orderBy) .distinct(true).build(); assertThat(this.visitor.visit(aggregateGroupConcat), equalTo( - "LISTAGG(10.5, '''') WITHIN GROUP(ORDER BY \"test_column\" DESC NULLS FIRST, \"test_column2\")")); + "LISTAGG(1.05E1, '''') WITHIN GROUP(ORDER BY \"test_column\" DESC NULLS FIRST, \"test_column2\")")); } @Test @@ -263,7 +199,7 @@ void testVisitSqlFunctionAggregateInSelectList() throws AdapterException { .build(); final List arguments = List.of(new SqlColumn(1, columnMetadata)); final SqlFunctionAggregate sqlFunctionAggregate = new SqlFunctionAggregate(AVG, arguments, false); - final SqlNode selectList = SqlSelectList.createSelectStarSelectList(); + final SqlNode selectList = SqlSelectList.createAnyValueSelectList(); sqlFunctionAggregate.setParent(selectList); assertThat(this.visitor.visit(sqlFunctionAggregate), equalTo("CAST(AVG(\"test_column\") AS FLOAT)")); } @@ -346,7 +282,7 @@ void testVisitSqlFunctionScalar2(final ScalarFunction scalarFunction, final Stri @Test void testVisitSqlFunctionScalarInSelectList() throws AdapterException { - final SqlSelectList selectList = SqlSelectList.createSelectStarSelectList(); + final SqlSelectList selectList = SqlSelectList.createAnyValueSelectList(); final List arguments = List.of(new SqlLiteralString("test"), new SqlLiteralString("")); final SqlFunctionScalar sqlFunctionScalar = new SqlFunctionScalar(TANH, arguments); sqlFunctionScalar.setParent(selectList); @@ -386,33 +322,12 @@ void testSqlGeneratorWithLimitOffset() throws AdapterException { assertEquals(SqlNormalizer.normalizeSql(expectedSql), SqlNormalizer.normalizeSql(actualSql)); } - @Test - void testSqlGeneratorWithSelectStarAndOffset() throws AdapterException { - SqlStatementSelect node = (SqlStatementSelect) getTestSqlNode(); - node.getLimit().setOffset(5); - node = SqlStatementSelect.builder().selectList(SqlSelectList.createSelectStarSelectList()) - .fromClause(node.getFromClause()).whereClause(node.getWhereClause()).groupBy(node.getGroupBy()) - .having(node.getHaving()).orderBy(node.getOrderBy()).limit(node.getLimit()).build(); - final String expectedSql = "SELECT c0, c1 FROM (" + // - " SELECT LIMIT_SUBSELECT.*, ROWNUM ROWNUM_SUB FROM ( " + // - " SELECT \"USER_ID\" AS c0, \"URL\" AS c1 " + // - " FROM \"test_schema\".\"CLICKS\"" + // - " WHERE 1 < \"USER_ID\"" + // - " GROUP BY \"USER_ID\"" + // - " HAVING 1 < COUNT(\"URL\")" + // - " ORDER BY \"USER_ID\"" + // - " ) LIMIT_SUBSELECT WHERE ROWNUM <= 15 " + // - ") WHERE ROWNUM_SUB > 5"; - final String actualSql = this.visitor.visit(node); - assertEquals(SqlNormalizer.normalizeSql(expectedSql), SqlNormalizer.normalizeSql(actualSql)); - } - private static SqlNode getTestSqlNode() { - return new DialectTestData().getTestSqlNode(); + return new DialectTestData().getTestSqlNode(); } - + private static class DialectTestData { - private SqlNode getTestSqlNode() { + private SqlNode getTestSqlNode() { // SELECT USER_ID, count(URL) FROM CLICKS // WHERE 1 < USER_ID // GROUP BY USER_ID @@ -421,9 +336,9 @@ private SqlNode getTestSqlNode() { // LIMIT 10; final TableMetadata clicksMeta = getClicksTableMetadata(); final SqlTable fromClause = new SqlTable("CLICKS", clicksMeta); - final SqlSelectList selectList = SqlSelectList.createRegularSelectList( - List.of(new SqlColumn(0, clicksMeta.getColumns().get(0)), new SqlFunctionAggregate( - AggregateFunction.COUNT, List.of(new SqlColumn(1, clicksMeta.getColumns().get(1))), false))); + final SqlSelectList selectList = SqlSelectList.createRegularSelectList(List.of( + new SqlColumn(0, clicksMeta.getColumns().get(0)), new SqlFunctionAggregate(AggregateFunction.COUNT, + List.of(new SqlColumn(1, clicksMeta.getColumns().get(1))), false))); final SqlNode whereClause = new SqlPredicateLess(new SqlLiteralExactnumeric(BigDecimal.ONE), new SqlColumn(0, clicksMeta.getColumns().get(0))); final SqlExpressionList groupBy = new SqlGroupBy(List.of(new SqlColumn(0, clicksMeta.getColumns().get(0)))); @@ -437,7 +352,7 @@ private SqlNode getTestSqlNode() { .groupBy(groupBy).having(having).orderBy(orderBy).limit(limit).build(); } - private TableMetadata getClicksTableMetadata() { + private TableMetadata getClicksTableMetadata() { final ColumnAdapterNotesJsonConverter converter = ColumnAdapterNotesJsonConverter.getInstance(); final List columns = new ArrayList<>(); final ColumnAdapterNotes decimalAdapterNotes = ColumnAdapterNotes.builder() // diff --git a/src/test/java/com/exasol/adapter/dialects/oracle/OracleTableMetadataReaderTest.java b/src/test/java/com/exasol/adapter/dialects/oracle/OracleTableMetadataReaderTest.java index df6d799..4ff0065 100644 --- a/src/test/java/com/exasol/adapter/dialects/oracle/OracleTableMetadataReaderTest.java +++ b/src/test/java/com/exasol/adapter/dialects/oracle/OracleTableMetadataReaderTest.java @@ -1,14 +1,14 @@ package com.exasol.adapter.dialects.oracle; import static com.exasol.adapter.jdbc.TableMetadataMockUtils.*; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Collections; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import org.junit.jupiter.api.BeforeEach; @@ -52,7 +52,7 @@ void testTablesInTrashBinAreNotMapped() throws SQLException { mockTableName(this.tablesMock, TABLE_A, TABLE_B, "BIN$TRASHED"); mockTableWithColumnsOfType(this.tablesMock, this.columnMetadataReaderMock, TABLE_A, DataType.createBool()); mockTableWithColumnsOfType(this.tablesMock, this.columnMetadataReaderMock, TABLE_B, DataType.createBool()); - final List tableNames = this.reader.mapTables(this.tablesMock, Optional.empty()) // + final List tableNames = this.reader.mapTables(this.tablesMock, Collections.emptyList()) // .stream() // .map(TableMetadata::getName) // .collect(Collectors.toList());