Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for unquoted Postgres identifiers (fix for #74) #75

Merged
merged 12 commits into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Types;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.*;

import com.exasol.adapter.jdbc.ColumnAdapterNotes;
import com.exasol.adapter.jdbc.JdbcAdapterProperties;
Expand Down Expand Up @@ -40,7 +36,7 @@ public String getTableCatalogAndSchemaSeparator() {
}

@Override
public MappedTable mapTable(final ResultSet tables) throws SQLException {
public MappedTable mapTable(final ResultSet tables, List<String> ignoreErrorList) throws SQLException {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
String commentString = tables.getString("REMARKS");
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
if (commentString == null) {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
commentString = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.List;
import java.util.Map;

import com.exasol.adapter.capabilities.Capabilities;
Expand Down Expand Up @@ -153,9 +154,10 @@ public String getTableComment() {
* @param tables A jdbc Resultset for the
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
* {@link DatabaseMetaData#getTables(String, String, String, String[])}
* call, pointing to the current table.
* @param ignoreErrorList
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
* @return An instance of {@link MappedTable} describing the mapped table.
*/
public MappedTable mapTable(ResultSet tables) throws SQLException;
public MappedTable mapTable(ResultSet tables, List<String> ignoreErrorList) throws SQLException;
snehlsen marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param columns A jdbc Resultset for the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.sql.SQLException;
import java.sql.Types;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;

import com.exasol.adapter.capabilities.AggregateFunctionCapability;
Expand Down Expand Up @@ -331,7 +332,7 @@ public SchemaOrCatalogSupport supportsJdbcSchemas() {
}

@Override
public MappedTable mapTable(final ResultSet tables) throws SQLException {
public MappedTable mapTable(final ResultSet tables, List<String> ignoreErrorList) throws SQLException {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
final String tableName = tables.getString("TABLE_NAME");
if (tableName.startsWith("BIN$")) {
// In case of Oracle we may see deleted tables with strange names
Expand All @@ -341,7 +342,7 @@ public MappedTable mapTable(final ResultSet tables) throws SQLException {
System.out.println("Skip table: " + tableName);
return MappedTable.createIgnoredTable();
} else {
return super.mapTable(tables);
return super.mapTable(tables, ignoreErrorList);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.exasol.adapter.dialects.impl;

import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Types;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;

import com.exasol.adapter.capabilities.AggregateFunctionCapability;
Expand All @@ -20,6 +22,9 @@
import com.exasol.adapter.sql.ScalarFunction;

public class PostgreSQLSqlDialect extends AbstractSqlDialect {

public static final String POSTGRES_IGNORE_UPPERCASE_TABLES = "POSTGRES_IGNORE_UPPERCASE_TABLES";

public PostgreSQLSqlDialect(final SqlDialectContext context) {
super(context);
}
Expand Down Expand Up @@ -298,6 +303,24 @@ public DataType dialectSpecificMapJdbcType(final JdbcTypeDescription jdbcTypeDes
return colType;
}

@Override
public MappedTable mapTable(final ResultSet tables, List<String> ignoreErrorList) throws SQLException {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
final String tableName = tables.getString("TABLE_NAME");
if (ignoreErrorList.contains(POSTGRES_IGNORE_UPPERCASE_TABLES)) {
return super.mapTable(tables, ignoreErrorList);
}
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
if (containsUppercaseCharacter(tableName)) {
throw new IllegalArgumentException("Table " + tableName + " cannot be used in virtual schema. " +
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
"Set property IGNORE_ERROR_LIST to POSTGRES_IGNORE_UPPERCASE_TABLES to enforce schema creation.");
} else {
return super.mapTable(tables, ignoreErrorList);
}
}

private boolean containsUppercaseCharacter(String tableName) {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
return !tableName.equals(tableName.toLowerCase());
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public Map<ScalarFunction, String> getScalarFunctionAliases() {

Expand Down Expand Up @@ -345,7 +368,7 @@ public IdentifierCaseHandling getQuotedIdentifierHandling() {

@Override
public String applyQuote(final String identifier) {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
return "\"" + identifier.replace("\"", "\"\"") + "\"";
return "\"" + identifier.toLowerCase().replace("\"", "\"\"") + "\"";
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ private static SchemaMetadata readMetadata(final SchemaMetadataInfo meta, final
return JdbcMetadataReader.readRemoteMetadata(connection.getAddress(), connection.getUser(),
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
connection.getPassword(), catalog, schema, tables,
JdbcAdapterProperties.getSqlDialectName(meta.getProperties()),
JdbcAdapterProperties.getExceptionHandlingMode(meta.getProperties()));
JdbcAdapterProperties.getExceptionHandlingMode(meta.getProperties()),
JdbcAdapterProperties.getIgnoreErrorList(meta.getProperties()));
}

private static String handleRefresh(final RefreshRequest request, final ExaMetadata meta)
Expand Down Expand Up @@ -165,7 +166,8 @@ private static String handleSetProperty(final SetPropertiesRequest request, fina
connection.getUser(), connection.getPassword(), JdbcAdapterProperties.getCatalog(newSchemaMeta),
JdbcAdapterProperties.getSchema(newSchemaMeta), tableFilter,
JdbcAdapterProperties.getSqlDialectName(newSchemaMeta),
JdbcAdapterProperties.getExceptionHandlingMode(newSchemaMeta));
JdbcAdapterProperties.getExceptionHandlingMode(newSchemaMeta),
JdbcAdapterProperties.getIgnoreErrorList(newSchemaMeta));
return ResponseJsonSerializer.makeSetPropertiesResponse(remoteMeta);
}
return ResponseJsonSerializer.makeSetPropertiesResponse(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import com.exasol.ExaConnectionAccessException;
import com.exasol.ExaConnectionInformation;
Expand Down Expand Up @@ -43,6 +44,7 @@ public final class JdbcAdapterProperties {
static final String PROP_EXCLUDED_CAPABILITIES = "EXCLUDED_CAPABILITIES";
static final String PROP_EXCEPTION_HANDLING = "EXCEPTION_HANDLING";
static final String PROP_LOG_LEVEL = "LOG_LEVEL";
static final String PROP_IGNORE_ERROR_LIST = "IGNORE_ERROR_LIST";
snehlsen marked this conversation as resolved.
Show resolved Hide resolved

private static final String DEFAULT_LOG_LEVEL = "INFO";

Expand All @@ -64,6 +66,12 @@ private static String getProperty(final Map<String, String> properties, final St
}
}

public static List<String> getIgnoreErrorList(final Map<String, String> properties) {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
String ignoreErrors = getProperty(properties, PROP_IGNORE_ERROR_LIST, "");
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
List<String> ignoreErrorsList = Arrays.asList(ignoreErrors.split(","));
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
return ignoreErrorsList.stream().map(error -> error.trim().toUpperCase()).collect(Collectors.toList());
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
}

public static String getCatalog(final Map<String, String> properties) {
return getProperty(properties, PROP_CATALOG_NAME, "");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class JdbcMetadataReader {

public static SchemaMetadata readRemoteMetadata(final String connectionString, final String user,
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
final String password, String catalog, String schema, final List<String> tableFilter,
final String dialectName, final JdbcAdapterProperties.ExceptionHandlingMode exceptionMode)
final String dialectName, final JdbcAdapterProperties.ExceptionHandlingMode exceptionMode, final List<String> ignoreErrorList)
throws SQLException, AdapterException {
assert (catalog != null);
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
assert (schema != null);
Expand All @@ -45,7 +45,7 @@ public static SchemaMetadata readRemoteMetadata(final String connectionString, f

schema = findSchema(schema, dbMeta, dialect);
snehlsen marked this conversation as resolved.
Show resolved Hide resolved

final List<TableMetadata> tables = findTables(catalog, schema, tableFilter, dbMeta, dialect, exceptionMode);
final List<TableMetadata> tables = findTables(catalog, schema, tableFilter, dbMeta, dialect, exceptionMode, ignoreErrorList);

conn.close();
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
return new SchemaMetadata(SchemaAdapterNotes.serialize(schemaAdapterNotes), tables);
Expand Down Expand Up @@ -242,8 +242,8 @@ private static String findSchema(final String schema, final DatabaseMetaData dbM
}

private static List<TableMetadata> findTables(final String catalog, final String schema,
final List<String> tableFilter, final DatabaseMetaData dbMeta, final SqlDialect dialect,
final JdbcAdapterProperties.ExceptionHandlingMode exceptionMode) throws SQLException {
final List<String> tableFilter, final DatabaseMetaData dbMeta, final SqlDialect dialect,
final JdbcAdapterProperties.ExceptionHandlingMode exceptionMode, List<String> ignoreErrorList) throws SQLException {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
final List<TableMetadata> tables = new ArrayList<>();

final String[] supportedTableTypes = { "TABLE", "VIEW", "SYSTEM TABLE" };
Expand All @@ -252,7 +252,7 @@ private static List<TableMetadata> findTables(final String catalog, final String
final List<SqlDialect.MappedTable> tablesMapped = new ArrayList<>();
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
// List<String> tableComments = new ArrayList<>();
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
while (resTables.next()) {
final SqlDialect.MappedTable mappedTable = dialect.mapTable(resTables);
final SqlDialect.MappedTable mappedTable = dialect.mapTable(resTables, ignoreErrorList);
if (!mappedTable.isIgnored()) {
tablesMapped.add(mappedTable);
// tableComments.add(mappedTable.getTableComment());
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.junit.Assume;
Expand Down Expand Up @@ -308,7 +309,7 @@ public void testDifferentDataTypes()
final List<String> tables = new ArrayList<>(Arrays.asList(tableNames));
final SchemaMetadata meta = JdbcMetadataReader.readRemoteMetadata("jdbc:exa:" + getConfig().getExasolAddress(),
getConfig().getExasolUser(), getConfig().getExasolPassword(), "EXA_DB", "JDBC_ADAPTER_TEST_SCHEMA",
tables, ExasolSqlDialect.getPublicName(), getConfig().getExceptionHandlingMode());
tables, ExasolSqlDialect.getPublicName(), getConfig().getExceptionHandlingMode(), Collections.emptyList());
if (getConfig().isDebugOn()) {
System.out.println("Meta: " + SchemaMetadataSerializer.serialize(meta).build().toString());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.exasol.adapter.dialects.impl;

import com.exasol.adapter.dialects.SqlDialectContext;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import static org.mockito.Mockito.*;

import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import static org.junit.Assert.*;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.when;

public class PostgreSQLSqlDialectTest {
@Mock
SqlDialectContext sqlDialectContext;

snehlsen marked this conversation as resolved.
Show resolved Hide resolved

@Before
public void setUp() throws SQLException {

}

@Test
public void applyQuoteOnUpperCase() {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
PostgreSQLSqlDialect postgresDialect = new PostgreSQLSqlDialect(sqlDialectContext);
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
assertEquals("\"abc\"", postgresDialect.applyQuote("ABC"));
}

@Test
public void applyQuoteOnMixedCase() {
PostgreSQLSqlDialect postgresDialect = new PostgreSQLSqlDialect(sqlDialectContext);
assertEquals("\"abcde\"", postgresDialect.applyQuote("AbCde"));
}

@Test(expected = IllegalArgumentException.class)
public void mapTableWithUpperCaseCharacters() throws SQLException {
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
ResultSet resultSet = mock(ResultSet.class);
when(resultSet.getString("TABLE_NAME")).thenReturn("uPPer");
PostgreSQLSqlDialect postgresDialect = new PostgreSQLSqlDialect(sqlDialectContext);
postgresDialect.mapTable(resultSet, Collections.emptyList());
}

@Test
public void mapTableWithLowerCaseCharacters() throws SQLException {
ResultSet resultSet = mock(ResultSet.class);
when(resultSet.getString("TABLE_NAME")).thenReturn("lower");
PostgreSQLSqlDialect postgresDialect = new PostgreSQLSqlDialect(sqlDialectContext);
postgresDialect.mapTable(resultSet, Collections.emptyList());
}

@Test
public void mapTableWithIgnoreUppercaseCharactersError() throws SQLException {
ResultSet resultSet = mock(ResultSet.class);
when(resultSet.getString("TABLE_NAME")).thenReturn("Upper");
List<String> ignoreList = new ArrayList<>();
ignoreList.add("Dummy_Error");
ignoreList.add("POSTGRES_IGNORE_UPPERCASE_TABLES");
PostgreSQLSqlDialect postgresDialect = new PostgreSQLSqlDialect(sqlDialectContext);
postgresDialect.mapTable(resultSet, ignoreList);
}
}
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.Assert.assertEquals;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -246,4 +247,15 @@ public void testNoneAsExceptionValue() throws AdapterException {
JdbcAdapterProperties.getExceptionHandlingMode(properties));
JdbcAdapterProperties.checkPropertyConsistency(properties);
}

@Test
public void getIgnoreErrorList() {
Map<String, String> properties = new HashMap<>();
properties.put("IGNORE_ERROR_LIST", "ERrror_foo, error_bar , another_error");
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
List<String> expectedErrorList = new ArrayList<>();
expectedErrorList.add("ERRROR_FOO");
expectedErrorList.add("ERROR_BAR");
expectedErrorList.add("ANOTHER_ERROR");
assertEquals(expectedErrorList, JdbcAdapterProperties.getIgnoreErrorList(properties));
snehlsen marked this conversation as resolved.
Show resolved Hide resolved
}
}