Skip to content

Commit

Permalink
#108 Workaround for GROUP BY <integer> (#109)
Browse files Browse the repository at this point in the history
Co-authored-by: Christoph Kuhnke <[email protected]>
  • Loading branch information
kaklakariada and ckunki authored Jul 12, 2023
1 parent bd134d6 commit 7bc2aee
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 73 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ jobs:
strategy:
fail-fast: false
matrix:
docker_db_version: [ 7.1.17, 7.0.20 ]
docker_db_version: ["7.1.21"]
env:
DEFAULT_DB_VERSION: "7.1.17"
DEFAULT_DB_VERSION: "7.1.21"
runs-on: ubuntu-latest
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.docker_db_version }}
Expand All @@ -25,9 +25,9 @@ jobs:
- name: Set up JDK 11
uses: actions/setup-java@v3
with:
distribution: 'temurin'
distribution: "temurin"
java-version: 11
cache: 'maven'
cache: "maven"
- name: Cache SonarCloud packages
uses: actions/cache@v3
with:
Expand Down
34 changes: 19 additions & 15 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
{
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true,
"source.generate.finalModifiers": true,
"source.fixAll": true
},
"java.codeGeneration.useBlocks": true,
"java.saveActions.organizeImports": true,
"java.sources.organizeImports.starThreshold": 3,
"java.sources.organizeImports.staticStarThreshold": 3,
"java.test.config": {
"vmArgs": [
"-Djava.util.logging.config.file=src/test/resources/logging.properties"
]
},
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true,
"source.generate.finalModifiers": true,
"source.fixAll": true
},
"java.codeGeneration.useBlocks": true,
"java.saveActions.organizeImports": true,
"java.sources.organizeImports.starThreshold": 3,
"java.sources.organizeImports.staticStarThreshold": 3,
"java.test.config": {
"vmArgs": [
"-Djava.util.logging.config.file=src/test/resources/logging.properties"
]
},
"sonarlint.connectedMode.project": {
"connectionId": "exasol",
"projectKey": "com.exasol:exasol-virtual-schema"
}
}
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Find all the documentation in the [Virtual Schemas project][vs-doc].
## Information for Developers

* [Virtual Schema API Documentation][vs-api]
* [Developer Guide](./doc/developer_guide.md)

<!-- @formatter:off -->
[virtual-schemas-user-guide]: https://docs.exasol.com/database_concepts/virtual_schemas.htm
Expand Down
2 changes: 1 addition & 1 deletion dependencies.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions doc/changes/changelog.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions doc/changes/changes_7.1.4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Exasol Virtual Schema 7.1.4, released 2023-07-12

Code name: Fix Issue With Integer Constants in `GROUP BY`

## Summary

This release fixes an issue with queries using `DISTINCT` with integer constants. The Exasol SQL processor turns `DISTINCT <integer>` into `GROUP BY <integer>` before push-down as an optimization. The adapter must not feed this back as Exasol interprets integers in `GROUP BY` clauses as column numbers which could lead to invalid results or the following error:

```
42000:Wrong column number. Too small value 0 as select list column reference in GROUP BY (smallest possible value is 1)
```

To fix this, Exasol VS now replaces integer constants in `GROUP BY` clauses with a constant string.

Please that you can still safely use `GROUP BY <column-number>` in your original query, since Exasol internally converts this to `GROUP BY "<column-name>"`, so that the virtual schema adapter can tell both situations apart.

## Bugfixes

* #108: Fixed issue with integer constants in `GROUP BY`

## Dependency Updates

### Compile Dependency Updates

* Updated `com.exasol:virtual-schema-common-jdbc:11.0.0` to `11.0.1`

### Test Dependency Updates

* Updated `com.exasol:exasol-testcontainers:6.6.0` to `6.6.1`
* Updated `com.exasol:virtual-schema-common-jdbc:11.0.0` to `11.0.1`

### Plugin Dependency Updates

* Updated `org.apache.maven.plugins:maven-assembly-plugin:3.3.0` to `3.6.0`
5 changes: 5 additions & 0 deletions doc/developer_guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Developer Guide

## Enable Debug Output

To enable debug output for the virtual schema adapter you can set [system properties defined by test-db-builder-java](https://github.com/exasol/test-db-builder-java/blob/main/doc/user_guide/user_guide.md#debug-output).
2 changes: 1 addition & 1 deletion doc/dialects/exasol.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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>/virtual-schema-dist-11.0.0-exasol-7.1.3.jar;
%jar /buckets/<BFS service>/<bucket>/virtual-schema-dist-11.0.1-exasol-7.1.4.jar;
/
```

Expand Down
2 changes: 1 addition & 1 deletion pk_generated_parent.pom

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 4 additions & 32 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,13 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<artifactId>exasol-virtual-schema</artifactId>
<version>7.1.3</version>
<version>7.1.4</version>
<name>Exasol Virtual Schema</name>
<description>This projects contains the Exasol dialect for Exasol's Virtual Schema</description>
<url>https://github.com/exasol/exasol-virtual-schema/</url>
<properties>
<vscjdbc.version>11.0.0</vscjdbc.version>
<vscjdbc.version>11.0.1</vscjdbc.version>
</properties>
<distributionManagement>
<snapshotRepository>
<id>ossrh</id>
<url>https://oss.sonatype.org/content/repositories/snapshots</url>
</snapshotRepository>
<repository>
<id>ossrh</id>
<url>https://oss.sonatype.org/service/local/staging/deploy/maven2/</url>
</repository>
</distributionManagement>
<dependencies>
<dependency>
<groupId>com.exasol</groupId>
Expand Down Expand Up @@ -66,7 +56,7 @@
<dependency>
<groupId>com.exasol</groupId>
<artifactId>exasol-testcontainers</artifactId>
<version>6.6.0</version>
<version>6.6.1</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -116,13 +106,8 @@
</plugin>
<plugin>
<artifactId>maven-assembly-plugin</artifactId>
<version>3.3.0</version>
<configuration>
<descriptors>
<descriptor>src/assembly/all-dependencies.xml</descriptor>
</descriptors>
<finalName>virtual-schema-dist-${vscjdbc.version}-exasol-${project.version}</finalName>
<appendAssemblyId>false</appendAssemblyId>
</configuration>
<executions>
<execution>
Expand Down Expand Up @@ -159,19 +144,6 @@
<useModulePath>false</useModulePath>
</configuration>
</plugin>
<plugin>
<groupId>org.sonatype.ossindex.maven</groupId>
<artifactId>ossindex-maven-plugin</artifactId>
<configuration>
<excludeVulnerabilityIds>
<!-- This is a vulnerability in the transitive netty test dependency introduced by the
AWS SDK and in turn by exasol-test-setup-abstraction-java. AWS notes that they handle
netty in a way where this cannot be exploited.
See https://github.com/aws/aws-sdk-java-v2/issues/3263#issuecomment-1163600532 -->
<exclude>sonatype-2020-0026</exclude>
</excludeVulnerabilityIds>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
Expand All @@ -190,7 +162,7 @@
<parent>
<artifactId>exasol-virtual-schema-generated-parent</artifactId>
<groupId>com.exasol</groupId>
<version>7.1.3</version>
<version>7.1.4</version>
<relativePath>pk_generated_parent.pom</relativePath>
</parent>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,6 @@ private static AdapterScript installVirtualSchemaAdapter(final ExasolSchema adap
.build();
}

protected static boolean exasolVersionSupportsFingerprintInAddress() {
final ExasolDockerImageReference imageReference = EXASOL.getDockerImageReference();
if (imageReference.getMajor() >= 8) {
return true;
}
return (imageReference.getMajor() >= 7) && (imageReference.getMinor() >= 1);
}

@AfterAll
static void afterAll() throws SQLException {
dropAll(adapterScript, adapterSchema);
Expand All @@ -108,11 +100,8 @@ private ConnectionDefinition createAdapterConnectionDefinition(final User user)

private String getJdbcUrl() {
final int port = EXASOL.getDefaultInternalDatabasePort();
if (exasolVersionSupportsFingerprintInAddress()) {
final String fingerprint = EXASOL.getTlsCertificateFingerprint().orElseThrow();
return "jdbc:exa:localhost/" + fingerprint + ":" + port;
}
return "jdbc:exa:localhost:" + port + ";validateservercertificate=0";
final String fingerprint = EXASOL.getTlsCertificateFingerprint().orElseThrow();
return "jdbc:exa:localhost/" + fingerprint + ":" + port;
}

@AfterEach
Expand Down Expand Up @@ -171,6 +160,13 @@ protected VirtualSchema createVirtualSchema(final Schema sourceSchema) {
.build();
}

/**
* Get properties for the virtual schema. Note: if you want to enable debug output, you can set <a href=
* "https://github.com/exasol/test-db-builder-java/blob/main/doc/user_guide/user_guide.md#debug-output">system
* properties defined by test-db-builder-java</a>.
*
* @return properties for the virtual schema
*/
private Map<String, String> getVirtualSchemaProperties() {
return getConnectionSpecificVirtualSchemaProperties();
}
Expand Down Expand Up @@ -855,6 +851,42 @@ void testWildcards() throws SQLException {
table().row("A", "VARCHAR(20) UTF8", null, null, null).matches());
}

@Test
@DisplayName("Verify DISTINCT with integer literal")
void testDistinctWithIntegerLiteral() throws SQLException {
final Table table = createSingleColumnTable("INT") //
.insert(1).insert(1).insert(2).insert(3);
final VirtualSchema virtualSchema = createVirtualSchema(this.sourceSchema);
try {
assertThat(
query("SELECT DISTINCT c1, 0 AS attr from "
+ virtualSchema.getFullyQualifiedName() + "." + table.getName()),
table("BIGINT", "SMALLINT") //
.row(1L, (short) 0).row(2L, (short) 0).row(3L, (short) 0) //
.matchesInAnyOrder());
} finally {
virtualSchema.drop();
}
}

@Test
@DisplayName("Verify GROUP BY with column number reference")
void testGroupByWithColumnNumber() throws SQLException {
final Table table = createSingleColumnTable("INT") //
.insert(1).insert(1).insert(2).insert(3);
final VirtualSchema virtualSchema = createVirtualSchema(this.sourceSchema);
try {
assertThat(
query("SELECT c1, count(c1) as count from "
+ virtualSchema.getFullyQualifiedName() + "." + table.getName() + " group by 1"),
table("BIGINT", "BIGINT") //
.row(1L, 2L).row(2L, 1L).row(3L, 1L) //
.matchesInAnyOrder());
} finally {
virtualSchema.drop();
}
}

boolean isVersionOrHigher(final int majorVersion, final int minorVersion, final int fixVersion) {
final ExasolDockerImageReference version = EXASOL.getDockerImageReference();
final long comparableImageVersion = calculatedComparableVersion((version.hasMajor() ? version.getMajor() : 0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ void beforeEach() {
}

private String getTargetAddress() {
final String fingerprint = exasolVersionSupportsFingerprintInAddress()
? "/" + EXASOL.getTlsCertificateFingerprint().orElseThrow()
: "";
return "127.0.0.1" + fingerprint + ":" + EXASOL.getDefaultInternalDatabasePort();
return "127.0.0.1" + "/" + EXASOL.getTlsCertificateFingerprint().orElseThrow() + ":"
+ EXASOL.getDefaultInternalDatabasePort();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import java.nio.file.Path;

public final class IntegrationTestConfiguration {
private static final String DEFAULT_DOCKER_DB_REFERENCE = "7.1.17";
private static final String DEFAULT_DOCKER_DB_REFERENCE = "7.1.21";
/**
* Do not use MavenProjectVersionGetter here to enable reference checker to check if reference points to the latest
* version.
*/
public static final String VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION = "virtual-schema-dist-11.0.0-exasol-7.1.3.jar";
public static final String VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION = "virtual-schema-dist-11.0.1-exasol-7.1.4.jar";
public static final Path PATH_TO_VIRTUAL_SCHEMAS_JAR = Path.of("target", VIRTUAL_SCHEMAS_JAR_NAME_AND_VERSION);

private IntegrationTestConfiguration() {
Expand Down

0 comments on commit 7bc2aee

Please sign in to comment.