Skip to content

Commit

Permalink
* #401: Fixed review findings
Browse files Browse the repository at this point in the history
  • Loading branch information
AnastasiiaSergienko committed Nov 9, 2020
1 parent b39cae2 commit 741887d
Show file tree
Hide file tree
Showing 40 changed files with 358 additions and 170 deletions.
7 changes: 6 additions & 1 deletion doc/changes/changes_4.0.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,9 @@ Code name:
* Updated com.exasol:test-db-builder-java:1.0.1 to 1.1.0
* Updated com.exasol:hamcrest-resultset-matcher:1.1.1 to 1.2.1
* Updated nl.jqno.equalsverifier:equalsverifier:3.4.3 to 3.5
* Updated mysql:mysql-connector-java:8.0.21 to 8.0.22
* Updated mysql:mysql-connector-java:8.0.21 to 8.0.22
* Updated org.testcontainers:junit-jupiter:1.14.3 to 1.15.0
* Updated org.testcontainers:mssqlserver:1.14.3 to 1.15.0
* Updated org.testcontainers:mysql:1.14.3 to 1.15.0
* Updated org.testcontainers:oracle-xe:1.14.3 to 1.15.0
* Updated org.testcontainers:postgresql:1.14.3 to 1.15.0
3 changes: 1 addition & 2 deletions doc/dialects/athena.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ You need to specify the following settings when adding the JDBC driver via EXAOp
Please refer to the [documentation on configuring JDBC connections to Athena](https://docs.aws.amazon.com/athena/latest/ug/connect-with-jdbc.html) for details.

IMPORTANT: The latest Athena driver requires to **Disable Security Manager**.
It is necessary because JDBC driver requires Java permissions which we do not grant by default.
Please keep in mind that it's not safe to disable the security manager.
It is necessary because JDBC driver requires Java permissions which we do not grant by default.

## Uploading the JDBC Driver to EXAOperation

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<java.version>11</java.version>
<surefire.and.failsafe.plugin.version>3.0.0-M4</surefire.and.failsafe.plugin.version>
<vscjdbc.version>6.1.0</vscjdbc.version>
<org.testcontainers.version>1.14.3</org.testcontainers.version>
<org.testcontainers.version>1.15.0</org.testcontainers.version>
<sonar.coverage.jacoco.xmlReportPaths>target/site/jacoco/jacoco.xml,target/site/jacoco-it/jacoco.xml
</sonar.coverage.jacoco.xmlReportPaths>
</properties>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.exasol.adapter.dialects.athena;

import com.exasol.db.Identifier;

import java.util.Objects;

import com.exasol.db.Identifier;

/**
* Represents an identifier in the Athena database.
*/
Expand Down Expand Up @@ -71,8 +71,12 @@ private static boolean validateCharacter(final int codePoint) {

@Override
public boolean equals(final Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (this == o) {
return true;
}
if (!(o instanceof AthenaIdentifier)) {
return false;
}
final AthenaIdentifier that = (AthenaIdentifier) o;
return Objects.equals(this.id, that.id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public boolean requiresSchemaQualifiedTableNames(final SqlGenerationContext cont
}

@Override
// https://docs.aws.amazon.com/athena/latest/ug/tables-databases-columns-names.html
public String applyQuote(final String identifier) {
return AthenaIdentifier.of(identifier).quote();
}
Expand All @@ -116,6 +117,11 @@ public NullSorting getDefaultNullSorting() {
return NullSorting.NULLS_SORTED_AT_END;
}

@Override
public String getStringLiteral(final String value) {
return super.quoteLiteralStringWithSingleQuote(value);
}

@Override
protected RemoteMetadataReader createRemoteMetadataReader() {
try {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ public StructureElementSupport supportsJdbcSchemas() {
}

@Override
// https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical
public String applyQuote(final String identifier) {
return BigQueryIdentifier.of(identifier).quote();
return "`" + identifier.replace("`", "\\`") + "`";
}

@Override
Expand Down Expand Up @@ -156,7 +157,7 @@ public String getStringLiteral(final String value) {
if (value == null) {
return "NULL";
} else {
return "'" + value.replace("'", "\\'") + "'";
return "'" + value.replace("\\", "\\\\").replace("'", "\\'") + "'";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,9 @@ public StructureElementSupport supportsJdbcSchemas() {
}

@Override
@SuppressWarnings("squid:S1185")
// https://www.ibm.com/support/knowledgecenter/SSEPEK_10.0.0/sqlref/src/tpc/db2z_sqlidentifiers.html
public String applyQuote(final String identifier) {
return super.applyQuote(identifier);
return super.quoteIdentifierWithDoubleQuotes(identifier);
}

@Override
Expand All @@ -124,4 +123,9 @@ public SqlNodeVisitor<String> getSqlGenerationVisitor(final SqlGenerationContext
public NullSorting getDefaultNullSorting() {
return NullSorting.NULLS_SORTED_AT_END;
}

@Override
public String getStringLiteral(final String value) {
return super.quoteLiteralStringWithSingleQuote(value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public NullSorting getDefaultNullSorting() {
}
}

@Override
public String getStringLiteral(final String value) {
return this.quoteLiteralStringWithSingleQuote(value);
}

@Override
protected RemoteMetadataReader createRemoteMetadataReader() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ public NullSorting getDefaultNullSorting() {
return NullSorting.NULLS_SORTED_LOW;
}

@Override
public String getStringLiteral(final String value) {
return super.quoteLiteralStringWithSingleQuote(value);
}

@Override
public SqlNodeVisitor<String> getSqlGenerationVisitor(final SqlGenerationContext context) {
return new HiveSqlGenerationVisitor(this, context);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.exasol.adapter.dialects.impala;

import java.util.Objects;

import com.exasol.db.Identifier;

/**
* Represents an identifier in the Impala database.
*/
public class ImpalaIdentifier implements Identifier {
private final String id;

private ImpalaIdentifier(final String id) {
this.id = id;
}

/**
* Get the quoted identifier as a {@link String}.
*
* @return quoted identifier
*/
@Override
public String quote() {
return "`" + this.id + "`";
}

/**
* Create a new {@link ImpalaIdentifier}.
*
* @param id the identifier as {@link String}
* @return new {@link ImpalaIdentifier} instance
*/
public static ImpalaIdentifier of(final String id) {
if (validate(id)) {
return new ImpalaIdentifier(id);
} else {
throw new AssertionError("E-ID-6: Unable to create identifier \"" + id //
+ "\" because it contains illegal characters." //
+ " For information about valid identifiers, please refer to" //
+ " https://docs.cloudera.com/documentation/enterprise/latest/topics/impala_identifiers.html");
}
}

private static boolean validate(final String id) {
return !id.contains("`");
}

@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
if (!(o instanceof ImpalaIdentifier)) {
return false;
}
final ImpalaIdentifier that = (ImpalaIdentifier) o;
return Objects.equals(this.id, that.id);
}

@Override
public int hashCode() {
return Objects.hash(this.id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public StructureElementSupport supportsJdbcSchemas() {
@Override
// https://docs.cloudera.com/documentation/enterprise/latest/topics/impala_identifiers.html
public String applyQuote(final String identifier) {
return "`" + identifier.replace("`", "``") + "`";
return ImpalaIdentifier.of(identifier).quote();
}

@Override
Expand Down Expand Up @@ -139,7 +139,7 @@ public String getStringLiteral(final String value) {
if (value == null) {
return "NULL";
} else {
return "'" + value.replace("'", "\\'") + "'";
return "'" + value.replace("\\", "\\\\").replace("'", "\\'") + "'";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,8 @@ public StructureElementSupport supportsJdbcSchemas() {
return StructureElementSupport.NONE;
}

/**
* @see <a href="http://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_ansi_quotes">ANSI quotes (MySQL
* reference manual)</a>
*/
@Override
// https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
public String applyQuote(final String identifier) {
return "`" + identifier.replace("`", "``") + "`";
}
Expand All @@ -107,6 +104,11 @@ public NullSorting getDefaultNullSorting() {
return NullSorting.NULLS_SORTED_AT_END;
}

@Override
public String getStringLiteral(final String value) {
return super.quoteLiteralStringWithSingleQuote(value);
}

@Override
protected RemoteMetadataReader createRemoteMetadataReader() {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.exasol.adapter.dialects.oracle;

import java.util.Objects;

import com.exasol.db.Identifier;

/**
* Represents an identifier in the Oracle database.
*/
public class OracleIdentifier implements Identifier {
private final String id;

private OracleIdentifier(final String id) {
this.id = id;
}

/**
* Get the quoted identifier as a {@link String}.
*
* @return quoted identifier
*/
@Override
public String quote() {
return "\"" + this.id + "\"";
}

/**
* Create a new {@link OracleIdentifier}.
*
* @param id the identifier as {@link String}
* @return new {@link OracleIdentifier} instance
*/
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");
}
}

private static boolean validate(final String id) {
return !id.contains("\"");
}

@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
if (!(o instanceof OracleIdentifier)) {
return false;
}
final OracleIdentifier that = (OracleIdentifier) o;
return Objects.equals(this.id, that.id);
}

@Override
public int hashCode() {
return Objects.hash(this.id);
}
}

0 comments on commit 741887d

Please sign in to comment.