Bugs/6 exception with timestamps (#7)

* #5: updated vs-common versions * #6: throw an exception when Timestamp with local time zone is used * #6: added integration tests and documented new property Co-authored-by: Sebastian Bär <[email protected]>
exasol · Apr 21, 2020 · 106292f · 106292f
1 parent 02ef9d2
commit 106292f
Show file tree

Hide file tree

Showing 9 changed files with 208 additions and 77 deletions.
diff --git a/README.md b/README.md
@@ -26,8 +26,8 @@ This projects contains the Exasol dialect for [Exasol's Virtual Schema][virtual-
 
 ### Information for Users
 
-* [User guide][user-guide]
 * [Exasol dialect](doc/dialects/exasol.md)
+* [User guide][user-guide]
 
 Find all the documentation in the [Virtual Schemas project][vs-doc].
 
@@ -65,6 +65,6 @@ Running the Virtual Schema requires a Java Runtime version 11 or later.
 | [Maven Source Plugin](https://maven.apache.org/plugins/maven-source-plugin/)        | Creating a source code JAR                             | Apache License 2.0            |
 | [Maven Surefire Plugin](https://maven.apache.org/surefire/maven-surefire-plugin/)   | Unit testing                                           | Apache License 2.0            |
 
-[user-guide]: https://github.com/exasol/virtual-schemas/blob/master/doc/user-guide/user_guide.md
+[user-guide]: https://github.com/exasol/virtual-schemas/blob/master/doc/user-guide/user_guide.md#using-the-adapter
 [virtual-schemas]: https://github.com/exasol/virtual-schemas
 [vs-doc]: https://github.com/exasol/virtual-schemas/tree/master/doc
diff --git a/doc/dialects/exasol.md b/doc/dialects/exasol.md
@@ -18,7 +18,7 @@ The SQL statement below creates the adapter script, defines the Java class that
 ```sql
 CREATE JAVA ADAPTER SCRIPT SCHEMA_FOR_VS_SCRIPT.ADAPTER_SCRIPT_EXASOL AS
     %scriptclass com.exasol.adapter.RequestDispatcher;
-    %jar /buckets/<BFS service>/<bucket>/exasol-virtual-schema-dist-1.0.1.jar;
+    %jar /buckets/<BFS service>/<bucket>/exasol-virtual-schema-dist-2.0.0.jar;
 /
 ```
 
@@ -64,6 +64,14 @@ CREATE VIRTUAL SCHEMA <virtual schema name>
     SCHEMA_NAME     = '<schema name>';
 ```
 
+## Known limitations
+
+* Using literals and constant expressions with datatype `TIMESTAMP WITH LOCAL TIME ZONE` in Virtual Schemas 
+can produce an incorrect results. We recommend using 'TIMESTAMP' instead. If you are willing to take the risk
+and want to use `TIMESTAMP WITH LOCAL TIME ZONE` anyway, please, create a Virtual Schema with the following
+additional property `IGNORE_ERRORS = 'TIMESTAMP_WITH_LOCAL_TIME_ZONE_USAGE'`. 
+We also recommend to set Exasol system `time_zone` to UTC while working with `TIMESTAMP WITH LOCAL TIME ZONE`.
+
 ## Supported Capabilities
 
 The Exasol SQL dialect supports all capabilities that are supported by the virtual schema framework.

diff --git a/pom.xml b/pom.xml
@@ -9,15 +9,15 @@
     <description>This projects contains the Exasol dialect for Exasol's Virtual Schema</description>
     <url>https://github.com/exasol/exasol-virtual-schema</url>
     <properties>
-        <product.version>1.0.1</product.version>
+        <product.version>2.0.0</product.version>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
         <java.version>11</java.version>
         <junit.version>5.6.1</junit.version>
         <junit.platform.version>1.6.1</junit.platform.version>
         <surefire.and.failsafe.plugin.version>3.0.0-M4</surefire.and.failsafe.plugin.version>
-        <vscjava.version>8.0.2</vscjava.version>
-        <vscjdbc.version>3.0.2</vscjdbc.version>
+        <vscjava.version>9.0.0</vscjava.version>
+        <vscjdbc.version>4.0.0</vscjdbc.version>
         <sonar.coverage.jacoco.xmlReportPaths>target/site/jacoco/jacoco.xml,target/site/jacoco-it/jacoco.xml
         </sonar.coverage.jacoco.xmlReportPaths>
         <gpg.skip>true</gpg.skip>

diff --git a/src/main/java/com/exasol/adapter/dialects/exasol/ExasolSqlDialect.java b/src/main/java/com/exasol/adapter/dialects/exasol/ExasolSqlDialect.java
@@ -14,18 +14,21 @@
 import com.exasol.adapter.capabilities.*;
 import com.exasol.adapter.dialects.*;
 import com.exasol.adapter.jdbc.*;
+import com.exasol.adapter.sql.SqlNodeVisitor;
 
 /**
  * Exasol SQL dialect.
  */
 public class ExasolSqlDialect extends AbstractSqlDialect {
+    static final String EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH = "TIMESTAMP_WITH_LOCAL_TIME_ZONE_USAGE";
+
     static final String NAME = "EXASOL";
     private static final Capabilities CAPABILITIES = createCapabilityList();
     private static final List<String> SUPPORTED_PROPERTIES = Arrays.asList(SQL_DIALECT_PROPERTY,
             CONNECTION_NAME_PROPERTY, CONNECTION_STRING_PROPERTY, USERNAME_PROPERTY, PASSWORD_PROPERTY,
             CATALOG_NAME_PROPERTY, SCHEMA_NAME_PROPERTY, TABLE_FILTER_PROPERTY, EXASOL_IMPORT_PROPERTY,
             EXASOL_CONNECTION_STRING_PROPERTY, IS_LOCAL_PROPERTY, EXCLUDED_CAPABILITIES_PROPERTY,
-            DEBUG_ADDRESS_PROPERTY, LOG_LEVEL_PROPERTY);
+            DEBUG_ADDRESS_PROPERTY, LOG_LEVEL_PROPERTY, IGNORE_ERRORS_PROPERTY);
 
     /**
      * Create a new instance of the {@link ExasolSqlDialect}.
@@ -50,16 +53,18 @@ public String getName() {
 
     private static Capabilities createCapabilityList() {
         return Capabilities.builder() //
-                .addMain(SELECTLIST_PROJECTION, SELECTLIST_EXPRESSIONS, FILTER_EXPRESSIONS, AGGREGATE_SINGLE_GROUP,
-                        AGGREGATE_GROUP_BY_COLUMN, AGGREGATE_GROUP_BY_EXPRESSION, AGGREGATE_GROUP_BY_TUPLE,
-                        AGGREGATE_HAVING, ORDER_BY_COLUMN, ORDER_BY_EXPRESSION, LIMIT, LIMIT_WITH_OFFSET, JOIN,
-                        JOIN_TYPE_INNER, JOIN_TYPE_LEFT_OUTER, JOIN_TYPE_RIGHT_OUTER, JOIN_TYPE_FULL_OUTER,
-                        JOIN_CONDITION_EQUI) //
-                .addLiteral(LiteralCapability.values()) //
-                .addPredicate(PredicateCapability.values()) //
-                .addAggregateFunction(AggregateFunctionCapability.values()) //
-                .addScalarFunction(ScalarFunctionCapability.values()) //
-                .build();
+                           .addMain(SELECTLIST_PROJECTION, SELECTLIST_EXPRESSIONS, FILTER_EXPRESSIONS,
+                                   AGGREGATE_SINGLE_GROUP,
+                                   AGGREGATE_GROUP_BY_COLUMN, AGGREGATE_GROUP_BY_EXPRESSION, AGGREGATE_GROUP_BY_TUPLE,
+                                   AGGREGATE_HAVING, ORDER_BY_COLUMN, ORDER_BY_EXPRESSION, LIMIT, LIMIT_WITH_OFFSET,
+                                   JOIN,
+                                   JOIN_TYPE_INNER, JOIN_TYPE_LEFT_OUTER, JOIN_TYPE_RIGHT_OUTER, JOIN_TYPE_FULL_OUTER,
+                                   JOIN_CONDITION_EQUI) //
+                           .addLiteral(LiteralCapability.values()) //
+                           .addPredicate(PredicateCapability.values()) //
+                           .addAggregateFunction(AggregateFunctionCapability.values()) //
+                           .addScalarFunction(ScalarFunctionCapability.values()) //
+                           .build();
     }
 
     @Override
@@ -148,4 +153,18 @@ public void validateProperties() throws PropertyValidationException {
     protected List<String> getSupportedProperties() {
         return SUPPORTED_PROPERTIES;
     }
+
+    @Override
+    public SqlNodeVisitor<String> getSqlGenerationVisitor(final SqlGenerationContext context) {
+        return new ExasolSqlGenerationVisitor(this, context);
+    }
+
+    /**
+     * Check if the adapter should ignore the usage of the literal timestamp with local time zone.
+     *
+     * @return <code>true</code> if the property is enabled
+     */
+    boolean isTimestampWithLocalTimeZoneEnabled() {
+        return this.properties.getIgnoredErrors().contains(EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH);
+    }
 }
diff --git a/src/main/java/com/exasol/adapter/dialects/exasol/ExasolSqlGenerationVisitor.java b/src/main/java/com/exasol/adapter/dialects/exasol/ExasolSqlGenerationVisitor.java
@@ -0,0 +1,44 @@
+package com.exasol.adapter.dialects.exasol;
+
+import static com.exasol.adapter.dialects.exasol.ExasolSqlDialect.EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH;
+
+import java.util.logging.Logger;
+
+import com.exasol.adapter.dialects.*;
+import com.exasol.adapter.sql.SqlLiteralTimestampUtc;
+
+/**
+ * This class generates SQL queries for the {@link ExasolSqlDialect}.
+ */
+public class ExasolSqlGenerationVisitor extends SqlGenerationVisitor {
+    private static final Logger LOGGER = Logger.getLogger(ExasolSqlGenerationVisitor.class.getName());
+
+    /**
+     * Creates a new instance of the {@link ExasolSqlGenerationVisitor}.
+     *
+     * @param dialect {@link ExasolSqlDialect} dialect
+     * @param context SQL generation context
+     */
+    ExasolSqlGenerationVisitor(final SqlDialect dialect,
+            final SqlGenerationContext context) {
+        super(dialect, context);
+    }
+
+    @Override
+    public String visit(final SqlLiteralTimestampUtc literal) {
+        final ExasolSqlDialect exasolSqlDialect = (ExasolSqlDialect) getDialect();
+        if (exasolSqlDialect.isTimestampWithLocalTimeZoneEnabled()) {
+            LOGGER.info("IGNORE_ERRORS = '" + EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH + "' property is enabled.");
+            return super.visit(literal);
+        } else {
+            throw new UnsupportedOperationException(
+                    "Attention! Using literals and constant expressions with datatype `TIMESTAMP WITH LOCAL TIME ZONE` " +
+                            "in Virtual Schemas can produce an incorrect results. We recommend using 'TIMESTAMP' instead. " +
+                            "If you are willing to take the risk and want to use `TIMESTAMP WITH LOCAL TIME ZONE` anyway, please, " +
+                            "create a Virtual Schema with the following additional property " +
+                            "IGNORE_ERRORS = '" + EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH + "'. " +
+                            "We also recommend to set Exasol system `time_zone` " +
+                            "to UTC while working with `TIMESTAMP WITH LOCAL TIME ZONE`.");
+        }
+    }
+}
diff --git a/src/test/java/com/exasol/adapter/dialects/exasol/ExasolFromExaQueryRewriterTest.java b/src/test/java/com/exasol/adapter/dialects/exasol/ExasolFromExaQueryRewriterTest.java
@@ -1,87 +1,74 @@
 package com.exasol.adapter.dialects.exasol;
 
 import static com.exasol.adapter.AdapterProperties.*;
+import static com.exasol.adapter.dialects.exasol.ExasolProperties.EXASOL_CONNECTION_STRING_PROPERTY;
 import static com.exasol.adapter.dialects.exasol.ExasolProperties.EXASOL_IMPORT_PROPERTY;
 import static com.exasol.reflect.ReflectionUtils.getMethodReturnViaReflection;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.sql.Connection;
 import java.sql.SQLException;
-import java.util.HashMap;
+import java.util.Map;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
-import com.exasol.*;
 import com.exasol.adapter.AdapterException;
 import com.exasol.adapter.AdapterProperties;
 import com.exasol.adapter.dialects.*;
-import com.exasol.adapter.jdbc.BaseRemoteMetadataReader;
 import com.exasol.adapter.jdbc.ConnectionFactory;
 import com.exasol.adapter.sql.TestSqlStatementFactory;
 
+@ExtendWith(MockitoExtension.class)
 class ExasolFromExaQueryRewriterTest extends AbstractQueryRewriterTestBase {
     @BeforeEach
     void beforeEach() {
-        this.exaConnectionInformation = mock(ExaConnectionInformation.class);
-        this.exaMetadata = mock(ExaMetadata.class);
-        this.rawProperties = new HashMap<>();
         this.statement = TestSqlStatementFactory.createSelectOneFromDual();
     }
 
     @Test
-    void testRewriteWithJdbcConnection() throws AdapterException, SQLException, ExaConnectionAccessException {
-        mockExasolNamedConnection();
+    void testRewriteWithJdbcConnection(@Mock final ConnectionFactory connectionFactoryMock)
+            throws AdapterException, SQLException {
         final Connection connectionMock = mockConnection();
-        final ConnectionFactory connectionFactoryMock = mock(ConnectionFactory.class);
         when(connectionFactoryMock.getConnection()).thenReturn(connectionMock);
-        setConnectionNameProperty();
-        final AdapterProperties properties = new AdapterProperties(this.rawProperties);
-        final SqlDialect dialect = new ExasolSqlDialect(connectionFactoryMock, properties);
-        final BaseRemoteMetadataReader metadataReader = new BaseRemoteMetadataReader(connectionMock, properties);
+        final AdapterProperties properties = new AdapterProperties(Map.of("CONNECTION_NAME", CONNECTION_NAME));
+        final SqlDialectFactory dialectFactory = new ExasolSqlDialectFactory();
+        final SqlDialect dialect = dialectFactory.createSqlDialect(connectionFactoryMock, properties);
+        final ExasolMetadataReader metadataReader = new ExasolMetadataReader(connectionMock, properties);
         final QueryRewriter queryRewriter = new ExasolJdbcQueryRewriter(dialect, metadataReader, connectionFactoryMock);
-        assertThat(queryRewriter.rewrite(this.statement, this.exaMetadata, properties),
+        assertThat(queryRewriter.rewrite(this.statement, EXA_METADATA, properties),
                 equalTo("IMPORT INTO (c1 DECIMAL(18, 0)) FROM JDBC AT " + CONNECTION_NAME
                         + " STATEMENT 'SELECT 1 FROM \"DUAL\"'"));
     }
 
     @Test
     void testRewriteLocal() throws AdapterException, SQLException {
-        setIsLocalProperty();
-        final AdapterProperties properties = new AdapterProperties(this.rawProperties);
+        final AdapterProperties properties = new AdapterProperties(Map.of(IS_LOCAL_PROPERTY, "true"));
         final SqlDialect dialect = new ExasolSqlDialect(null, properties);
         final QueryRewriter queryRewriter = new ExasolLocalQueryRewriter(dialect);
-        assertThat(queryRewriter.rewrite(this.statement, this.exaMetadata, properties),
-                equalTo("SELECT 1 FROM \"DUAL\""));
-    }
-
-    private void setIsLocalProperty() {
-        this.rawProperties.put(IS_LOCAL_PROPERTY, "true");
+        assertThat(queryRewriter.rewrite(this.statement, EXA_METADATA, properties), equalTo("SELECT 1 FROM \"DUAL\""));
     }
 
     @Test
     void testRewriteToImportFromExaWithConnectionDetailsInProperties() throws AdapterException, SQLException {
-        setImportFromExaProperty();
-        this.rawProperties.put(CONNECTION_STRING_PROPERTY, "irrelevant");
-        this.rawProperties.put(USERNAME_PROPERTY, "alibaba");
-        this.rawProperties.put(PASSWORD_PROPERTY, "open sesame");
-        this.rawProperties.put(ExasolProperties.EXASOL_CONNECTION_STRING_PROPERTY, "localhost:7861");
-        final AdapterProperties properties = new AdapterProperties(this.rawProperties);
+        final AdapterProperties properties = new AdapterProperties(Map.of(EXASOL_IMPORT_PROPERTY, "true", //
+                CONNECTION_STRING_PROPERTY, "irrelevant", //
+                USERNAME_PROPERTY, "alibaba", //
+                PASSWORD_PROPERTY, "open sesame", //
+                EXASOL_CONNECTION_STRING_PROPERTY, "localhost:7861"));
         final SqlDialect dialect = new ExasolSqlDialect(null, properties);
         final QueryRewriter queryRewriter = new ExasolFromExaQueryRewriter(dialect, null, null);
-        assertThat(queryRewriter.rewrite(this.statement, this.exaMetadata, properties),
+        assertThat(queryRewriter.rewrite(this.statement, EXA_METADATA, properties),
                 equalTo("IMPORT FROM EXA AT 'localhost:7861' USER 'alibaba' IDENTIFIED BY 'open sesame'"
                         + " STATEMENT 'SELECT 1 FROM \"DUAL\"'"));
     }
 
-    private void setImportFromExaProperty() {
-        this.rawProperties.put(EXASOL_IMPORT_PROPERTY, "true");
-    }
-
     @Test
     void testConnectionDefinitionBuilderClass() {
         final SqlDialect dialect = new ExasolSqlDialect(null, AdapterProperties.emptyProperties());

diff --git a/src/test/java/com/exasol/adapter/dialects/exasol/ExasolSqlDialectIT.java b/src/test/java/com/exasol/adapter/dialects/exasol/ExasolSqlDialectIT.java
@@ -1,9 +1,9 @@
 package com.exasol.adapter.dialects.exasol;
 
+import static com.exasol.adapter.dialects.exasol.ExasolSqlDialect.EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH;
 import static com.exasol.matcher.ResultSetMatcher.matchesResultSet;
 import static java.util.Calendar.AUGUST;
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -34,7 +34,7 @@
 @Testcontainers
 class ExasolSqlDialectIT {
     private static final Logger LOGGER = LoggerFactory.getLogger(ExasolSqlDialectIT.class);
-    private static final String VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION = "exasol-virtual-schema-dist-1.0.1.jar";
+    private static final String VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION = "exasol-virtual-schema-dist-2.0.0.jar";
     private static final Path PATH_TO_VIRTUAL_SCHEMAS_JAR = Path.of("target", VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION);
     private static final String SCHEMA_EXASOL = "SCHEMA_EXASOL";
     private static final String ADAPTER_SCRIPT_EXASOL = "ADAPTER_SCRIPT_EXASOL";
@@ -54,7 +54,7 @@ class ExasolSqlDialectIT {
     @Container
     private static final ExasolContainer<? extends ExasolContainer<?>> container = new ExasolContainer<>(
             ExasolContainerConstants.EXASOL_DOCKER_IMAGE_REFERENCE) //
-                    .withLogConsumer(new Slf4jLogConsumer(LOGGER));
+            .withLogConsumer(new Slf4jLogConsumer(LOGGER));
     private static Statement statement;
     private static Connection connection;
 
@@ -169,7 +169,7 @@ private static void createConnection() throws SQLException {
     }
 
     private static void createVirtualSchema(final String virtualSchemaName, final String schemaName,
-            final Optional<String> additionalParameters) throws SQLException {
+                                            final Optional<String> additionalParameters) throws SQLException {
         final StringBuilder builder = new StringBuilder();
         builder.append("CREATE VIRTUAL SCHEMA ");
         builder.append(virtualSchemaName);
@@ -322,7 +322,7 @@ void assertUnquotedMixedCaseTableIsNotFound() {
     }
 
     @ParameterizedTest
-    @ValueSource(strings = { "Column1", "column2" })
+    @ValueSource(strings = {"Column1", "column2"})
     void assertUnquotedMixedCaseColumnIsNotFound(final String columnName) {
         final SQLException exception = assertThrows(SQLException.class, () -> statement.executeQuery("SELECT "
                 + columnName + " FROM \"" + VIRTUAL_SCHEMA_EXA_MIXED_CASE + "\".\"" + TABLE_MIXED_CASE + "\""));
@@ -756,4 +756,23 @@ void testFullOuterJoin(final String virtualSchemaName) throws SQLException {
                 + " a FULL OUTER JOIN  " + virtualSchemaName + "." + TABLE_JOIN_2 + " b ON a.x=b.x ORDER BY a.x");
         assertThat(actual, matchesResultSet(expected));
     }
+
+    @Test
+    void testCreateVirtualSchemaWithIgnoreErrorsProperty() throws SQLException {
+        createVirtualSchema("VIRTUAL_SCHEMA_IGNORES_ERRORS", SCHEMA_EXASOL,
+                Optional.of("IS_LOCAL = 'true' IGNORE_ERRORS = '" + EXASOL_TIMESTAMP_WITH_LOCAL_TIME_ZONE_SWITCH + "'"));
+        assertThat(statement.executeQuery(
+                "SELECT NOW() - INTERVAL '1' MINUTE FROM VIRTUAL_SCHEMA_IGNORES_ERRORS." + TABLE_SIMPLE_VALUES),
+                instanceOf(ResultSet.class));
+    }
+
+    @Test
+    void testVirtualSchemaWithoutIgnoreErrorsPropertyThrowsException() {
+        final SQLException exception = assertThrows(SQLException.class, () ->
+                statement.execute("SELECT NOW() - INTERVAL '1' MINUTE FROM "
+                        + VIRTUAL_SCHEMA_EXA_LOCAL + "." + TABLE_SIMPLE_VALUES));
+        assertThat(exception.getMessage(),
+                containsString("Attention! Using literals and constant expressions with datatype " +
+                        "`TIMESTAMP WITH LOCAL TIME ZONE` in Virtual Schemas can produce an incorrect results"));
+    }
 }