This is an automated email from the ASF dual-hosted git repository. agingade pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push: new 3efb4da GEODE-4991: do no use hard coded quotes (#1728) 3efb4da is described below commit 3efb4da0fb79ad418e0b463e31dd31b23788696b Author: Darrel Schneider <dschnei...@pivotal.io> AuthorDate: Tue Apr 10 10:40:59 2018 -0700 GEODE-4991: do no use hard coded quotes (#1728) java.sql.DatabaseMetaData.getIdentifierQuoteString is now used when quoting identifiers. If this method returns null, an empty string, or a string with all spaces then no quoting will be done. --- .../internal/{ColumnValue.java => ColumnData.java} | 14 +- .../jdbc/internal/DataSourceManager.java | 3 +- .../{ColumnValue.java => EntryColumnData.java} | 41 ++---- .../geode/connectors/jdbc/internal/SqlHandler.java | 146 +++++++++++---------- .../jdbc/internal/SqlStatementFactory.java | 87 ++++++------ .../jdbc/internal/SqlToPdxInstanceCreator.java | 2 +- .../connectors/jdbc/internal/TableMetaData.java | 9 +- .../jdbc/internal/TableMetaDataManager.java | 6 +- .../jdbc/internal/TableMetaDataView.java | 10 +- .../geode/connectors/jdbc/JdbcDistributedTest.java | 26 +++- .../connectors/jdbc/MySqlJdbcDistributedTest.java | 13 +- .../jdbc/PostgresJdbcDistributedTest.java | 9 +- .../{ColumnValueTest.java => ColumnDataTest.java} | 15 +-- .../connectors/jdbc/internal/SqlHandlerTest.java | 30 +++-- .../jdbc/internal/SqlStatementFactoryTest.java | 34 +++-- .../TableMetaDataManagerIntegrationTest.java | 8 +- .../jdbc/internal/TableMetaDataManagerTest.java | 24 ++++ 17 files changed, 258 insertions(+), 219 deletions(-) diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnData.java similarity index 78% copy from geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java copy to geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnData.java index 581b975..e1dba74 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnData.java @@ -16,23 +16,17 @@ package org.apache.geode.connectors.jdbc.internal; import java.sql.JDBCType; -class ColumnValue { - private final boolean isKey; +class ColumnData { private final String columnName; private final Object value; private final int dataType; - ColumnValue(boolean isKey, String columnName, Object value, int dataType) { - this.isKey = isKey; + ColumnData(String columnName, Object value, int dataType) { this.columnName = columnName; this.value = value; this.dataType = dataType; } - boolean isKey() { - return isKey; - } - String getColumnName() { return columnName; } @@ -47,7 +41,7 @@ class ColumnValue { @Override public String toString() { - return "ColumnValue [isKey=" + isKey + ", columnName=" + columnName + ", value=" + value - + ", dataType=" + JDBCType.valueOf(dataType) + "]"; + return "ColumnData [columnName=" + columnName + ", value=" + value + ", dataType=" + + JDBCType.valueOf(dataType) + "]"; } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java index b27f1ff..fb4e41c 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/DataSourceManager.java @@ -40,8 +40,7 @@ class DataSourceManager { private void close(JdbcDataSource dataSource) { try { dataSource.close(); - } catch (Exception e) { - // TODO ignored for now; should it be logged? + } catch (Exception ignore) { } } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/EntryColumnData.java similarity index 53% rename from geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java rename to geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/EntryColumnData.java index 581b975..5630f03 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ColumnValue.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/EntryColumnData.java @@ -14,40 +14,25 @@ */ package org.apache.geode.connectors.jdbc.internal; -import java.sql.JDBCType; +import java.util.Collections; +import java.util.List; -class ColumnValue { - private final boolean isKey; - private final String columnName; - private final Object value; - private final int dataType; +class EntryColumnData { + private final ColumnData entryKeyColumnData; + private final List<ColumnData> entryValueColumnData; - ColumnValue(boolean isKey, String columnName, Object value, int dataType) { - this.isKey = isKey; - this.columnName = columnName; - this.value = value; - this.dataType = dataType; + EntryColumnData(ColumnData entryKeyColumnData, List<ColumnData> entryValueColumnData) { + this.entryKeyColumnData = entryKeyColumnData; + this.entryValueColumnData = + entryValueColumnData != null ? entryValueColumnData : Collections.emptyList(); } - boolean isKey() { - return isKey; + public ColumnData getEntryKeyColumnData() { + return entryKeyColumnData; } - String getColumnName() { - return columnName; + public List<ColumnData> getEntryValueColumnData() { + return entryValueColumnData; } - Object getValue() { - return value; - } - - int getDataType() { - return dataType; - } - - @Override - public String toString() { - return "ColumnValue [isKey=" + isKey + ", columnName=" + columnName + ", value=" + value - + ", dataType=" + JDBCType.valueOf(dataType) + "]"; - } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java index 8fc2c5e..806b3d6 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlHandler.java @@ -20,7 +20,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Types; import java.util.ArrayList; -import java.util.Collections; import java.util.Date; import java.util.List; @@ -65,12 +64,11 @@ public class SqlHandler { try (Connection connection = getConnection(connectionConfig)) { TableMetaDataView tableMetaData = this.tableMetaDataManager.getTableMetaDataView(connection, regionMapping.getRegionToTableName()); - String realTableName = tableMetaData.getTableName(); - List<ColumnValue> columnList = - getColumnToValueList(tableMetaData, regionMapping, key, null, Operation.GET); + EntryColumnData entryColumnData = + getEntryColumnData(tableMetaData, regionMapping, key, null, Operation.GET); try (PreparedStatement statement = - getPreparedStatement(connection, columnList, realTableName, Operation.GET)) { - try (ResultSet resultSet = executeReadQuery(statement, columnList)) { + getPreparedStatement(connection, tableMetaData, entryColumnData, Operation.GET)) { + try (ResultSet resultSet = executeReadQuery(statement, entryColumnData)) { InternalCache cache = (InternalCache) region.getRegionService(); SqlToPdxInstanceCreator sqlToPdxInstanceCreator = new SqlToPdxInstanceCreator(cache, regionMapping, resultSet, tableMetaData); @@ -81,9 +79,9 @@ public class SqlHandler { return result; } - private ResultSet executeReadQuery(PreparedStatement statement, List<ColumnValue> columnList) + private ResultSet executeReadQuery(PreparedStatement statement, EntryColumnData entryColumnData) throws SQLException { - setValuesInStatement(statement, columnList); + setValuesInStatement(statement, entryColumnData); return statement.executeQuery(); } @@ -106,41 +104,50 @@ public class SqlHandler { return connectionConfig; } - private void setValuesInStatement(PreparedStatement statement, List<ColumnValue> columnList) + private void setValuesInStatement(PreparedStatement statement, EntryColumnData entryColumnData) throws SQLException { int index = 0; - for (ColumnValue columnValue : columnList) { + for (ColumnData columnData : entryColumnData.getEntryValueColumnData()) { index++; - Object value = columnValue.getValue(); - if (value instanceof Character) { - Character character = ((Character) value); - // if null character, set to null string instead of a string with the null character - value = character == Character.valueOf((char) 0) ? null : character.toString(); - } else if (value instanceof Date) { - Date jdkDate = (Date) value; - switch (columnValue.getDataType()) { - case Types.DATE: - value = new java.sql.Date(jdkDate.getTime()); - break; - case Types.TIME: - case Types.TIME_WITH_TIMEZONE: - value = new java.sql.Time(jdkDate.getTime()); - break; - case Types.TIMESTAMP: - case Types.TIMESTAMP_WITH_TIMEZONE: - value = new java.sql.Timestamp(jdkDate.getTime()); - break; - default: - // no conversion needed - break; - } - } - if (value == null) { - statement.setNull(index, columnValue.getDataType()); - } else { - statement.setObject(index, value); + setValueOnStatement(statement, index, columnData); + } + + ColumnData keyColumnData = entryColumnData.getEntryKeyColumnData(); + index++; + setValueOnStatement(statement, index, keyColumnData); + } + + private void setValueOnStatement(PreparedStatement statement, int index, ColumnData columnData) + throws SQLException { + Object value = columnData.getValue(); + if (value instanceof Character) { + Character character = ((Character) value); + // if null character, set to null string instead of a string with the null character + value = character == Character.valueOf((char) 0) ? null : character.toString(); + } else if (value instanceof Date) { + Date jdkDate = (Date) value; + switch (columnData.getDataType()) { + case Types.DATE: + value = new java.sql.Date(jdkDate.getTime()); + break; + case Types.TIME: + case Types.TIME_WITH_TIMEZONE: + value = new java.sql.Time(jdkDate.getTime()); + break; + case Types.TIMESTAMP: + case Types.TIMESTAMP_WITH_TIMEZONE: + value = new java.sql.Timestamp(jdkDate.getTime()); + break; + default: + // no conversion needed + break; } } + if (value == null) { + statement.setNull(index, columnData.getDataType()); + } else { + statement.setObject(index, value); + } } public <K, V> void write(Region<K, V> region, Operation operation, K key, PdxInstance value) @@ -155,13 +162,12 @@ public class SqlHandler { try (Connection connection = getConnection(connectionConfig)) { TableMetaDataView tableMetaData = this.tableMetaDataManager.getTableMetaDataView(connection, regionMapping.getRegionToTableName()); - String realTableName = tableMetaData.getTableName(); - List<ColumnValue> columnList = - getColumnToValueList(tableMetaData, regionMapping, key, value, operation); + EntryColumnData entryColumnData = + getEntryColumnData(tableMetaData, regionMapping, key, value, operation); int updateCount = 0; try (PreparedStatement statement = - getPreparedStatement(connection, columnList, realTableName, operation)) { - updateCount = executeWriteStatement(statement, columnList); + getPreparedStatement(connection, tableMetaData, entryColumnData, operation)) { + updateCount = executeWriteStatement(statement, entryColumnData); } catch (SQLException e) { if (operation.isDestroy()) { throw e; @@ -176,8 +182,8 @@ public class SqlHandler { if (updateCount <= 0) { Operation upsertOp = getOppositeOperation(operation); try (PreparedStatement upsertStatement = - getPreparedStatement(connection, columnList, realTableName, upsertOp)) { - updateCount = executeWriteStatement(upsertStatement, columnList); + getPreparedStatement(connection, tableMetaData, entryColumnData, upsertOp)) { + updateCount = executeWriteStatement(upsertStatement, entryColumnData); } } @@ -189,60 +195,62 @@ public class SqlHandler { return operation.isUpdate() ? Operation.CREATE : Operation.UPDATE; } - private int executeWriteStatement(PreparedStatement statement, List<ColumnValue> columnList) + private int executeWriteStatement(PreparedStatement statement, EntryColumnData entryColumnData) throws SQLException { - setValuesInStatement(statement, columnList); + setValuesInStatement(statement, entryColumnData); return statement.executeUpdate(); } private PreparedStatement getPreparedStatement(Connection connection, - List<ColumnValue> columnList, String tableName, Operation operation) throws SQLException { - String sqlStr = getSqlString(tableName, columnList, operation); + TableMetaDataView tableMetaData, EntryColumnData entryColumnData, Operation operation) + throws SQLException { + String sqlStr = getSqlString(tableMetaData, entryColumnData, operation); return connection.prepareStatement(sqlStr); } - private String getSqlString(String tableName, List<ColumnValue> columnList, Operation operation) { - SqlStatementFactory statementFactory = new SqlStatementFactory(); + private String getSqlString(TableMetaDataView tableMetaData, EntryColumnData entryColumnData, + Operation operation) { + SqlStatementFactory statementFactory = + new SqlStatementFactory(tableMetaData.getIdentifierQuoteString()); + String tableName = tableMetaData.getTableName(); if (operation.isCreate()) { - return statementFactory.createInsertSqlString(tableName, columnList); + return statementFactory.createInsertSqlString(tableName, entryColumnData); } else if (operation.isUpdate()) { - return statementFactory.createUpdateSqlString(tableName, columnList); + return statementFactory.createUpdateSqlString(tableName, entryColumnData); } else if (operation.isDestroy()) { - return statementFactory.createDestroySqlString(tableName, columnList); + return statementFactory.createDestroySqlString(tableName, entryColumnData); } else if (operation.isGet()) { - return statementFactory.createSelectQueryString(tableName, columnList); + return statementFactory.createSelectQueryString(tableName, entryColumnData); } else { throw new InternalGemFireException("unsupported operation " + operation); } } - <K> List<ColumnValue> getColumnToValueList(TableMetaDataView tableMetaData, + <K> EntryColumnData getEntryColumnData(TableMetaDataView tableMetaData, RegionMapping regionMapping, K key, PdxInstance value, Operation operation) { String keyColumnName = tableMetaData.getKeyColumnName(); - ColumnValue keyColumnValue = - new ColumnValue(true, keyColumnName, key, tableMetaData.getColumnDataType(keyColumnName)); + ColumnData keyColumnData = + new ColumnData(keyColumnName, key, tableMetaData.getColumnDataType(keyColumnName)); + List<ColumnData> valueColumnData = null; - if (operation.isDestroy() || operation.isGet()) { - return Collections.singletonList(keyColumnValue); + if (operation.isCreate() || operation.isUpdate()) { + valueColumnData = createColumnDataList(tableMetaData, regionMapping, value); } - List<ColumnValue> result = createColumnValueList(tableMetaData, regionMapping, value); - result.add(keyColumnValue); - return result; + return new EntryColumnData(keyColumnData, valueColumnData); } - private List<ColumnValue> createColumnValueList(TableMetaDataView tableMetaData, + private List<ColumnData> createColumnDataList(TableMetaDataView tableMetaData, RegionMapping regionMapping, PdxInstance value) { - final String keyColumnName = tableMetaData.getKeyColumnName(); - List<ColumnValue> result = new ArrayList<>(); + List<ColumnData> result = new ArrayList<>(); for (String fieldName : value.getFieldNames()) { String columnName = regionMapping.getColumnNameForField(fieldName, tableMetaData); - if (columnName.equalsIgnoreCase(keyColumnName)) { + if (tableMetaData.getKeyColumnName().equals(columnName)) { continue; } - ColumnValue columnValue = new ColumnValue(false, columnName, value.getField(fieldName), + ColumnData columnData = new ColumnData(columnName, value.getField(fieldName), tableMetaData.getColumnDataType(columnName)); - result.add(columnValue); + result.add(columnData); } return result; } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactory.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactory.java index d5367ef..5087bbe 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactory.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactory.java @@ -14,65 +14,62 @@ */ package org.apache.geode.connectors.jdbc.internal; -import java.util.List; - class SqlStatementFactory { + private final String quote; + + public SqlStatementFactory(String identifierQuoteString) { + this.quote = identifierQuoteString; + } - String createSelectQueryString(String tableName, List<ColumnValue> columnList) { - assert columnList.size() == 1; - ColumnValue keyCV = columnList.get(0); - assert keyCV.isKey(); - return "SELECT * FROM " + tableName + " WHERE " + keyCV.getColumnName() + " = ?"; + String createSelectQueryString(String tableName, EntryColumnData entryColumnData) { + ColumnData keyCV = entryColumnData.getEntryKeyColumnData(); + return "SELECT * FROM " + quoteIdentifier(tableName) + " WHERE " + + quoteIdentifier(keyCV.getColumnName()) + " = ?"; } - String createDestroySqlString(String tableName, List<ColumnValue> columnList) { - assert columnList.size() == 1; - ColumnValue keyCV = columnList.get(0); - assert keyCV.isKey(); - return "DELETE FROM " + tableName + " WHERE " + keyCV.getColumnName() + " = ?"; + String createDestroySqlString(String tableName, EntryColumnData entryColumnData) { + ColumnData keyCV = entryColumnData.getEntryKeyColumnData(); + return "DELETE FROM " + quoteIdentifier(tableName) + " WHERE " + + quoteIdentifier(keyCV.getColumnName()) + " = ?"; } - String createUpdateSqlString(String tableName, List<ColumnValue> columnList) { - StringBuilder query = new StringBuilder("UPDATE " + tableName + " SET "); + String createUpdateSqlString(String tableName, EntryColumnData entryColumnData) { + StringBuilder query = new StringBuilder("UPDATE " + quoteIdentifier(tableName) + " SET "); int idx = 0; - for (ColumnValue column : columnList) { - if (!column.isKey()) { - idx++; - if (idx > 1) { - query.append(", "); - } - query.append(column.getColumnName()); - query.append(" = ?"); - } - } - for (ColumnValue column : columnList) { - if (column.isKey()) { - query.append(" WHERE "); - query.append(column.getColumnName()); - query.append(" = ?"); - // currently only support simple primary key with one column - break; + for (ColumnData column : entryColumnData.getEntryValueColumnData()) { + idx++; + if (idx > 1) { + query.append(", "); } + query.append(quoteIdentifier(column.getColumnName())); + query.append(" = ?"); } + + ColumnData keyColumnData = entryColumnData.getEntryKeyColumnData(); + query.append(" WHERE "); + query.append(quoteIdentifier(keyColumnData.getColumnName())); + query.append(" = ?"); + return query.toString(); } - String createInsertSqlString(String tableName, List<ColumnValue> columnList) { - StringBuilder columnNames = new StringBuilder("INSERT INTO " + tableName + " ("); + String createInsertSqlString(String tableName, EntryColumnData entryColumnData) { + StringBuilder columnNames = + new StringBuilder("INSERT INTO " + quoteIdentifier(tableName) + " ("); StringBuilder columnValues = new StringBuilder(" VALUES ("); - int columnCount = columnList.size(); - int idx = 0; - for (ColumnValue column : columnList) { - idx++; - columnNames.append(column.getColumnName()); - columnValues.append('?'); - if (idx != columnCount) { - columnNames.append(", "); - columnValues.append(","); - } + + for (ColumnData column : entryColumnData.getEntryValueColumnData()) { + columnNames.append(quoteIdentifier(column.getColumnName())).append(", "); + columnValues.append("?,"); } - columnNames.append(")"); - columnValues.append(")"); + + ColumnData keyColumnData = entryColumnData.getEntryKeyColumnData(); + columnNames.append(quoteIdentifier(keyColumnData.getColumnName())).append(")"); + columnValues.append("?)"); return columnNames.append(columnValues).toString(); } + + private String quoteIdentifier(String identifier) { + return quote + identifier + quote; + } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlToPdxInstanceCreator.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlToPdxInstanceCreator.java index 5b04755..d4e1cf9 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlToPdxInstanceCreator.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/SqlToPdxInstanceCreator.java @@ -55,7 +55,7 @@ class SqlToPdxInstanceCreator { for (int i = 1; i <= columnCount; i++) { String columnName = metaData.getColumnName(i); if (regionMapping.isPrimaryKeyInValue() - || !tableMetaData.getKeyColumnName().equalsIgnoreCase(columnName)) { + || !tableMetaData.getKeyColumnName().equals(columnName)) { String fieldName = regionMapping.getFieldNameForColumn(columnName, typeRegistry); FieldType fieldType = getFieldType(typeRegistry, fieldName); writeField(columnName, i, fieldName, fieldType); diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java index 532bcc4..301e8b1 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaData.java @@ -24,11 +24,13 @@ public class TableMetaData implements TableMetaDataView { private final String tableName; private final String keyColumnName; private final HashMap<String, Integer> columnNameToTypeMap; + private final String identifierQuoteString; - public TableMetaData(String tableName, String keyColumnName) { + public TableMetaData(String tableName, String keyColumnName, String quoteString) { this.tableName = tableName; this.keyColumnName = keyColumnName; this.columnNameToTypeMap = new HashMap<>(); + this.identifierQuoteString = quoteString; } @Override @@ -58,4 +60,9 @@ public class TableMetaData implements TableMetaDataView { public void addDataType(String columnName, int dataType) { this.columnNameToTypeMap.put(columnName, dataType); } + + @Override + public String getIdentifierQuoteString() { + return this.identifierQuoteString; + } } diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java index d821380..f743caa 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManager.java @@ -46,7 +46,11 @@ public class TableMetaDataManager { try (ResultSet tables = metaData.getTables(null, null, "%", null)) { String realTableName = getTableNameFromMetaData(tableName, tables); String key = getPrimaryKeyColumnNameFromMetaData(realTableName, metaData); - result = new TableMetaData(realTableName, key); + String quoteString = metaData.getIdentifierQuoteString(); + if (quoteString == null) { + quoteString = ""; + } + result = new TableMetaData(realTableName, key, quoteString); getDataTypesFromMetaData(realTableName, metaData, result); } } catch (SQLException e) { diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataView.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataView.java index a929ff6..189d794 100644 --- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataView.java +++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataView.java @@ -19,11 +19,13 @@ package org.apache.geode.connectors.jdbc.internal; import java.util.Set; public interface TableMetaDataView { - public String getTableName(); + String getTableName(); - public String getKeyColumnName(); + String getKeyColumnName(); - public int getColumnDataType(String columnName); + int getColumnDataType(String columnName); - public Set<String> getColumnNames(); + Set<String> getColumnNames(); + + String getIdentifierQuoteString(); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java index 12fb6fb..9c7225c 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcDistributedTest.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.io.Serializable; import java.sql.Connection; +import java.sql.DatabaseMetaData; import java.sql.DriverManager; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -103,17 +104,22 @@ public abstract class JdbcDistributedTest implements Serializable { server = startupRule.startServerVM(1, x -> x.withConnectionToLocator(locator.getPort()).withPDXReadSerialized()); Connection connection = getConnection(); + DatabaseMetaData metaData = connection.getMetaData(); + String quote = metaData.getIdentifierQuoteString(); Statement statement = connection.createStatement(); - createSupportedFieldsTable(statement, TABLE_NAME); + createSupportedFieldsTable(statement, TABLE_NAME, quote); } - protected abstract void createSupportedFieldsTable(Statement statement, String tableName) - throws SQLException; + protected abstract void createSupportedFieldsTable(Statement statement, String tableName, + String quote) throws SQLException; private void insertNullDataForAllSupportedFieldsTable(String key) throws SQLException { Connection connection = DriverManager.getConnection(connectionUrl); + DatabaseMetaData metaData = connection.getMetaData(); + String quote = metaData.getIdentifierQuoteString(); - String insertQuery = "Insert into " + TABLE_NAME + " values (" + "?,?,?,?,?,?,?,?,?,?,?,?,?)"; + String insertQuery = + "Insert into " + quote + TABLE_NAME + quote + " values (" + "?,?,?,?,?,?,?,?,?,?,?,?,?)"; System.out.println("### Query is :" + insertQuery); PreparedStatement statement = connection.prepareStatement(insertQuery); createNullStatement(key, statement); @@ -127,8 +133,11 @@ public abstract class JdbcDistributedTest implements Serializable { private void insertDataForAllSupportedFieldsTable(String key, ClassWithSupportedPdxFields data) throws SQLException { Connection connection = DriverManager.getConnection(connectionUrl); + DatabaseMetaData metaData = connection.getMetaData(); + String quote = metaData.getIdentifierQuoteString(); - String insertQuery = "Insert into " + TABLE_NAME + " values (" + "?,?,?,?,?,?,?,?,?,?,?,?,?)"; + String insertQuery = + "Insert into " + quote + TABLE_NAME + quote + " values (" + "?,?,?,?,?,?,?,?,?,?,?,?,?)"; System.out.println("### Query is :" + insertQuery); PreparedStatement statement = connection.prepareStatement(insertQuery); statement.setObject(1, key); @@ -155,11 +164,18 @@ public abstract class JdbcDistributedTest implements Serializable { private void closeDB() throws SQLException { try (Connection connection = DriverManager.getConnection(connectionUrl)) { + DatabaseMetaData metaData = connection.getMetaData(); + String quote = metaData.getIdentifierQuoteString(); try (Statement statement = connection.createStatement()) { try { statement.execute("Drop table " + TABLE_NAME); } catch (SQLException ignore) { } + + try { + statement.execute("Drop table " + quote + TABLE_NAME + quote); + } catch (SQLException ignore) { + } } } } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/MySqlJdbcDistributedTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/MySqlJdbcDistributedTest.java index 48de63c..7d79c70 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/MySqlJdbcDistributedTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/MySqlJdbcDistributedTest.java @@ -62,13 +62,14 @@ public class MySqlJdbcDistributedTest extends JdbcDistributedTest { } @Override - protected void createSupportedFieldsTable(Statement statement, String tableName) + protected void createSupportedFieldsTable(Statement statement, String tableName, String quote) throws SQLException { - statement.execute("CREATE TABLE " + tableName + " (id varchar(10) primary key not null, " - + "aboolean smallint, " + "abyte smallint, " + "ashort smallint, " + "anint int, " - + "along bigint, " + "aFloat float, " + "ADOUBLE double, " + "astring varchar(10), " - + "adate datetime, " + "anobject varchar(20), " + "abytearray blob(100), " - + "achar char(1))"); + statement.execute("CREATE TABLE " + quote + tableName + quote + " (" + quote + "id" + quote + + " varchar(10) primary key not null, " + "aboolean boolean, " + "abyte smallint, " + + "ashort smallint, " + "anint int, " + quote + "along" + quote + " bigint, " + quote + + "aFloat" + quote + " float, " + quote + "ADOUBLE" + quote + " double, " + + "astring varchar(10), " + "adate datetime, " + "anobject varchar(20), " + + "abytearray blob(100), " + "achar char(1))"); } @Override diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/PostgresJdbcDistributedTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/PostgresJdbcDistributedTest.java index 224fd9e..0ec4eb5 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/PostgresJdbcDistributedTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/PostgresJdbcDistributedTest.java @@ -60,11 +60,12 @@ public class PostgresJdbcDistributedTest extends JdbcDistributedTest { } @Override - protected void createSupportedFieldsTable(Statement statement, String tableName) + protected void createSupportedFieldsTable(Statement statement, String tableName, String quote) throws SQLException { - statement.execute("CREATE TABLE " + tableName + " (id varchar(10) primary key not null, " - + "aboolean boolean, " + "abyte smallint, " + "ashort smallint, " + "anint int, " - + "along bigint, " + "aFloat float, " + "ADOUBLE double precision, " + statement.execute("CREATE TABLE " + quote + tableName + quote + " (" + quote + "id" + quote + + " varchar(10) primary key not null, " + "aboolean boolean, " + "abyte smallint, " + + "ashort smallint, " + "anint int, " + quote + "along" + quote + " bigint, " + quote + + "aFloat" + quote + " float, " + quote + "ADOUBLE" + quote + " double precision, " + "astring varchar(10), " + "adate timestamp, " + "anobject varchar(20), " + "abytearray bytea, " + "achar char(1))"); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnDataTest.java similarity index 81% rename from geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java rename to geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnDataTest.java index c25dd25..36529f6 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnDataTest.java @@ -25,22 +25,17 @@ import org.junit.experimental.categories.Category; import org.apache.geode.test.junit.categories.UnitTest; @Category(UnitTest.class) -public class ColumnValueTest { +public class ColumnDataTest { private static final String COLUMN_NAME = "columnName"; private static final Object VALUE = new Object(); private static final JDBCType DATA_TYPE = JDBCType.VARCHAR; - private ColumnValue value; + private ColumnData value; @Before public void setup() { - value = new ColumnValue(true, COLUMN_NAME, VALUE, DATA_TYPE.getVendorTypeNumber()); - } - - @Test - public void isKeyReturnsCorrectValue() { - assertThat(value.isKey()).isTrue(); + value = new ColumnData(COLUMN_NAME, VALUE, DATA_TYPE.getVendorTypeNumber()); } @Test @@ -60,7 +55,7 @@ public class ColumnValueTest { @Test public void toStringContainsAllVariables() { - assertThat(value.toString()).contains(Boolean.toString(true)).contains(COLUMN_NAME) - .contains(VALUE.toString()).contains(DATA_TYPE.toString()); + assertThat(value.toString()).contains(COLUMN_NAME).contains(VALUE.toString()) + .contains(DATA_TYPE.toString()); } } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java index 1f11f11..b3d3f28 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlHandlerTest.java @@ -519,11 +519,12 @@ public class SqlHandlerTest { ResultSet primaryKeys = getPrimaryKeysMetaData(); when(primaryKeys.next()).thenReturn(true).thenReturn(false); - List<ColumnValue> columnValueList = - handler.getColumnToValueList(tableMetaDataView, regionMapping, key, value, Operation.GET); + EntryColumnData entryColumnData = + handler.getEntryColumnData(tableMetaDataView, regionMapping, key, value, Operation.GET); - assertThat(columnValueList).hasSize(1); - assertThat(columnValueList.get(0).getColumnName()).isEqualTo(KEY_COLUMN); + assertThat(entryColumnData.getEntryKeyColumnData()).isNotNull(); + assertThat(entryColumnData.getEntryValueColumnData()).isEmpty(); + assertThat(entryColumnData.getEntryKeyColumnData().getColumnName()).isEqualTo(KEY_COLUMN); } @Test @@ -535,12 +536,14 @@ public class SqlHandlerTest { when(primaryKeys.next()).thenReturn(true).thenReturn(false); when(value.getFieldNames()).thenReturn(Arrays.asList(KEY_COLUMN, nonKeyColumn)); - List<ColumnValue> columnValueList = handler.getColumnToValueList(tableMetaDataView, - regionMapping, key, value, Operation.UPDATE); + EntryColumnData entryColumnData = + handler.getEntryColumnData(tableMetaDataView, regionMapping, key, value, Operation.UPDATE); - assertThat(columnValueList).hasSize(2); - assertThat(columnValueList.get(0).getColumnName()).isEqualTo(nonKeyColumn); - assertThat(columnValueList.get(1).getColumnName()).isEqualTo(KEY_COLUMN); + assertThat(entryColumnData.getEntryKeyColumnData()).isNotNull(); + assertThat(entryColumnData.getEntryValueColumnData()).hasSize(1); + assertThat(entryColumnData.getEntryValueColumnData().get(0).getColumnName()) + .isEqualTo(nonKeyColumn); + assertThat(entryColumnData.getEntryKeyColumnData().getColumnName()).isEqualTo(KEY_COLUMN); } @Test @@ -548,11 +551,12 @@ public class SqlHandlerTest { ResultSet primaryKeys = getPrimaryKeysMetaData(); when(primaryKeys.next()).thenReturn(true).thenReturn(false); - List<ColumnValue> columnValueList = handler.getColumnToValueList(tableMetaDataView, - regionMapping, key, value, Operation.DESTROY); + EntryColumnData entryColumnData = + handler.getEntryColumnData(tableMetaDataView, regionMapping, key, value, Operation.DESTROY); - assertThat(columnValueList).hasSize(1); - assertThat(columnValueList.get(0).getColumnName()).isEqualTo(KEY_COLUMN); + assertThat(entryColumnData.getEntryKeyColumnData()).isNotNull(); + assertThat(entryColumnData.getEntryValueColumnData()).isEmpty(); + assertThat(entryColumnData.getEntryKeyColumnData().getColumnName()).isEqualTo(KEY_COLUMN); } private ResultSet getPrimaryKeysMetaData() throws SQLException { diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java index b8bdbc2..c88c7d7 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java @@ -29,26 +29,28 @@ import org.apache.geode.test.junit.categories.UnitTest; public class SqlStatementFactoryTest { private static final String TABLE_NAME = "testTable"; - private static final String KEY_COLUMN_NAME = "column1"; + private static final String KEY_COLUMN_NAME = "keyColumn"; + private static final String VALUE_COLUMN_1_NAME = "valueColumn1"; + private static final String VALUE_COLUMN_2_NAME = "valueColumn2"; - private List<ColumnValue> columnValues = new ArrayList<>(); - private SqlStatementFactory factory = new SqlStatementFactory(); + private EntryColumnData entryColumnData; + private SqlStatementFactory factory = new SqlStatementFactory(""); @Before public void setup() { - columnValues.add(new ColumnValue(false, "column0", null, 0)); - columnValues.add(new ColumnValue(true, KEY_COLUMN_NAME, null, 0)); - columnValues.add(new ColumnValue(false, "column2", null, 0)); + List<ColumnData> columnData = new ArrayList<>(); + columnData.add(new ColumnData(VALUE_COLUMN_1_NAME, null, 0)); + columnData.add(new ColumnData(VALUE_COLUMN_2_NAME, null, 0)); + ColumnData keyColumnData = new ColumnData(KEY_COLUMN_NAME, null, 0); + entryColumnData = new EntryColumnData(keyColumnData, columnData); } @Test public void getSelectQueryString() throws Exception { String expectedStatement = String.format("SELECT * FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME); - List<ColumnValue> keyColumn = new ArrayList<>(); - keyColumn.add(new ColumnValue(true, KEY_COLUMN_NAME, null, 0)); - String statement = factory.createSelectQueryString(TABLE_NAME, keyColumn); + String statement = factory.createSelectQueryString(TABLE_NAME, entryColumnData); assertThat(statement).isEqualTo(expectedStatement); } @@ -57,10 +59,8 @@ public class SqlStatementFactoryTest { public void getDestroySqlString() throws Exception { String expectedStatement = String.format("DELETE FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME); - List<ColumnValue> keyColumn = new ArrayList<>(); - keyColumn.add(new ColumnValue(true, KEY_COLUMN_NAME, null, 0)); - String statement = factory.createDestroySqlString(TABLE_NAME, keyColumn); + String statement = factory.createDestroySqlString(TABLE_NAME, entryColumnData); assertThat(statement).isEqualTo(expectedStatement); } @@ -68,10 +68,9 @@ public class SqlStatementFactoryTest { @Test public void getUpdateSqlString() throws Exception { String expectedStatement = String.format("UPDATE %s SET %s = ?, %s = ? WHERE %s = ?", - TABLE_NAME, columnValues.get(0).getColumnName(), columnValues.get(2).getColumnName(), - KEY_COLUMN_NAME); + TABLE_NAME, VALUE_COLUMN_1_NAME, VALUE_COLUMN_2_NAME, KEY_COLUMN_NAME); - String statement = factory.createUpdateSqlString(TABLE_NAME, columnValues); + String statement = factory.createUpdateSqlString(TABLE_NAME, entryColumnData); assertThat(statement).isEqualTo(expectedStatement); } @@ -79,10 +78,9 @@ public class SqlStatementFactoryTest { @Test public void getInsertSqlString() throws Exception { String expectedStatement = String.format("INSERT INTO %s (%s, %s, %s) VALUES (?,?,?)", - TABLE_NAME, columnValues.get(0).getColumnName(), columnValues.get(1).getColumnName(), - columnValues.get(2).getColumnName()); + TABLE_NAME, VALUE_COLUMN_1_NAME, VALUE_COLUMN_2_NAME, KEY_COLUMN_NAME); - String statement = factory.createInsertSqlString(TABLE_NAME, columnValues); + String statement = factory.createInsertSqlString(TABLE_NAME, entryColumnData); assertThat(statement).isEqualTo(expectedStatement); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerIntegrationTest.java index 15c9e93..ffbebde 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerIntegrationTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerIntegrationTest.java @@ -19,6 +19,7 @@ package org.apache.geode.connectors.jdbc.internal; import static org.assertj.core.api.Assertions.assertThat; import java.sql.Connection; +import java.sql.DatabaseMetaData; import java.sql.SQLException; import java.sql.Statement; import java.sql.Types; @@ -68,8 +69,11 @@ public abstract class TableMetaDataManagerIntegrationTest { protected abstract Connection getConnection() throws SQLException; protected void createTable() throws SQLException { - statement.execute("CREATE TABLE " + REGION_TABLE_NAME - + " (id VARCHAR(10) primary key not null, name VARCHAR(10), age int)"); + DatabaseMetaData metaData = connection.getMetaData(); + String quote = metaData.getIdentifierQuoteString(); + statement.execute("CREATE TABLE " + REGION_TABLE_NAME + " (" + quote + "id" + quote + + " VARCHAR(10) primary key not null," + quote + "name" + quote + " VARCHAR(10)," + quote + + "age" + quote + " int)"); } diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java index 5ab18b1..00f0c8b 100644 --- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java +++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/TableMetaDataManagerTest.java @@ -77,6 +77,30 @@ public class TableMetaDataManagerTest { } @Test + public void returnsDefaultQuoteString() throws Exception { + setupPrimaryKeysMetaData(); + when(primaryKeysResultSet.next()).thenReturn(true).thenReturn(false); + + TableMetaDataView data = tableMetaDataManager.getTableMetaDataView(connection, TABLE_NAME); + + assertThat(data.getIdentifierQuoteString()).isEqualTo(""); + verify(connection).getMetaData(); + } + + @Test + public void returnsQuoteStringFromMetaData() throws Exception { + setupPrimaryKeysMetaData(); + when(primaryKeysResultSet.next()).thenReturn(true).thenReturn(false); + String expectedQuoteString = "123"; + when(databaseMetaData.getIdentifierQuoteString()).thenReturn(expectedQuoteString); + + TableMetaDataView data = tableMetaDataManager.getTableMetaDataView(connection, TABLE_NAME); + + assertThat(data.getIdentifierQuoteString()).isEqualTo(expectedQuoteString); + verify(connection).getMetaData(); + } + + @Test public void secondCallDoesNotUseMetaData() throws Exception { setupPrimaryKeysMetaData(); when(primaryKeysResultSet.next()).thenReturn(true).thenReturn(false); -- To stop receiving notification emails like this one, please contact aging...@apache.org.