From df26a20d81d5b607c4e3a6d96c35eb7de9c17b61 Mon Sep 17 00:00:00 2001 From: yrizhkov Date: Sun, 17 Dec 2023 09:55:03 +0200 Subject: [PATCH] FMWK-288 Fix secondary index mixup from adjacent tables --- .../aerospike/jdbc/AerospikeConnection.java | 4 +- .../jdbc/AerospikeDatabaseMetadata.java | 69 ++++++++----------- .../aerospike/jdbc/AerospikeStatement.java | 6 +- .../jdbc/model/AerospikeSecondaryIndex.java | 13 ++-- .../jdbc/query/SelectQueryHandler.java | 40 +++++------ .../jdbc/sql/AerospikeResultSetMetaData.java | 5 +- .../aerospike/jdbc/DatabaseMetadataTest.java | 4 +- .../aerospike/jdbc/PreparedQueriesTest.java | 2 - .../com/aerospike/jdbc/SimpleQueriesTest.java | 2 - 9 files changed, 63 insertions(+), 82 deletions(-) diff --git a/src/main/java/com/aerospike/jdbc/AerospikeConnection.java b/src/main/java/com/aerospike/jdbc/AerospikeConnection.java index a250003..07e318b 100644 --- a/src/main/java/com/aerospike/jdbc/AerospikeConnection.java +++ b/src/main/java/com/aerospike/jdbc/AerospikeConnection.java @@ -331,8 +331,8 @@ public Struct createStruct(String typeName, Object[] attributes) throws SQLExcep } @Override - public String getSchema() throws SQLException { - return schema.get(); + public String getSchema() { + return null; } @Override diff --git a/src/main/java/com/aerospike/jdbc/AerospikeDatabaseMetadata.java b/src/main/java/com/aerospike/jdbc/AerospikeDatabaseMetadata.java index 1f3a0ff..a895e85 100644 --- a/src/main/java/com/aerospike/jdbc/AerospikeDatabaseMetadata.java +++ b/src/main/java/com/aerospike/jdbc/AerospikeDatabaseMetadata.java @@ -25,7 +25,6 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; -import java.util.function.Function; import java.util.logging.Logger; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -55,10 +54,10 @@ public class AerospikeDatabaseMetadata implements DatabaseMetaData, SimpleWrappe private final Connection connection; private final String dbBuild; private final String dbEdition; + private final List catalogs; private final Map> tables; private final Map> catalogIndexes; - private final Map secondaryIndexes; private final AerospikeSchemaBuilder schemaBuilder; private final Cache resultSetMetaDataCache; @@ -102,9 +101,6 @@ public AerospikeDatabaseMetadata(String url, IAerospikeClient client, AerospikeC ); }); }); - secondaryIndexes = catalogIndexes.values().stream() - .flatMap(Collection::stream) - .collect(Collectors.toMap(AerospikeSecondaryIndex::toKey, Function.identity())); schemaBuilder = new AerospikeSchemaBuilder(client, connection.getConfiguration().getDriverPolicy()); resultSetMetaDataCache = CacheBuilder.newBuilder().build(); @@ -763,7 +759,7 @@ public ResultSet getTables(String catalog, String schemaPattern, String tableNam public ResultSet getSchemas() { return new ListRecordSet(null, "system", "schemas", systemColumns(new String[]{"TABLE_SCHEM", "TABLE_CATALOG"}, new int[]{VARCHAR, VARCHAR}), - catalogs.stream().map(ns -> Arrays.asList(ns, ns)).collect(toList())); + catalogs.stream().map(ns -> Arrays.asList("", ns)).collect(toList())); } @Override @@ -783,28 +779,27 @@ public ResultSet getTableTypes() { @Override public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { - logger.info(() -> format("AerospikeDatabaseMetadata getColumns; %s, %s, %s, %s", catalog, - schemaPattern, tableNamePattern, columnNamePattern)); + logger.info(() -> format("getColumns: %s, %s, %s, %s", catalog, schemaPattern, tableNamePattern, + columnNamePattern)); Pattern tableNameRegex = isNullOrEmpty(tableNamePattern) ? null : Pattern.compile(tableNamePattern.replace("%", ".*")); - final String namespace = catalog == null ? schemaPattern : catalog; - final List mds; - if (namespace == null) { - mds = tables.entrySet().stream() + final List resultSetMetaDataList; + if (catalog == null) { + resultSetMetaDataList = tables.entrySet().stream() .flatMap(p -> p.getValue().stream().map(t -> getMetadata(p.getKey(), t))) .collect(toList()); } else { - mds = tables.getOrDefault(namespace, Collections.emptyList()).stream() + resultSetMetaDataList = tables.getOrDefault(catalog, Collections.emptyList()).stream() .filter(t -> tableNameRegex == null || tableNameRegex.matcher(t).matches()) - .map(t -> getMetadata(namespace, t)) + .map(t -> getMetadata(catalog, t)) .collect(toList()); } List> result = new ArrayList<>(); - for (ResultSetMetaData md : mds) { - int n = md.getColumnCount(); - for (int i = 1; i <= n; i++) { + for (ResultSetMetaData md : resultSetMetaDataList) { + int columnCount = md.getColumnCount(); + for (int i = 1; i <= columnCount; i++) { result.add(asList("".equals(tableNamePattern) ? "" : md.getCatalogName(i), null, md.getTableName(1), md.getColumnName(i), md.getColumnType(i), md.getColumnTypeName(i), 0, 0, 0, 0, columnNullable, null, null, md.getColumnType(i), 0, @@ -963,14 +958,17 @@ public ResultSet getTypeInfo() { @Override public ResultSet getIndexInfo(String catalog, String schema, String table, boolean unique, boolean approximate) { + logger.info(() -> format("getIndexInfo: %s, %s, %s", catalog, schema, table)); Stream secondaryIndexStream; if (catalog == null) { - secondaryIndexStream = catalogIndexes.entrySet().stream().flatMap(p -> p.getValue().stream()); + secondaryIndexStream = catalogIndexes.entrySet().stream() + .flatMap(p -> p.getValue().stream()); } else { - secondaryIndexStream = getOrDefault(catalogIndexes, catalog, Collections.emptyList()).stream(); + secondaryIndexStream = getOrDefault(catalogIndexes, catalog, Collections.emptyList()).stream() + .filter(i -> i.getNamespace().equals(catalog)); } - final Iterable> indicesData = secondaryIndexStream - .filter(i -> i.getNamespace().equals(schema) && i.getSet().equals(table)) + final Iterable> indexData = secondaryIndexStream + .filter(i -> i.getSet().equals(table)) .map(this::indexInfoAsList) .collect(Collectors.toList()); @@ -982,11 +980,11 @@ public ResultSet getIndexInfo(String catalog, String schema, String table, boole VARCHAR, BIGINT, BIGINT, VARCHAR}; return new ListRecordSet(null, "system", "index_info", - systemColumns(columns, sqlTypes), indicesData); + systemColumns(columns, sqlTypes), indexData); } - public Map getSecondaryIndexes() { - return secondaryIndexes; + public Collection getSecondaryIndexes(String catalog) { + return catalogIndexes.get(catalog); } @Override @@ -1085,18 +1083,16 @@ public boolean supportsGetGeneratedKeys() { @Override public ResultSet getSuperTypes(String catalog, String schemaPattern, String typeNamePattern) { - List> types = asList( - asList(catalog, null, "list", null, null, Object.class.getName()), - asList(catalog, null, "map", null, null, Object.class.getName()), - asList(catalog, null, "GeoJSON", null, null, Object.class.getName()) - ); + List> typeNames = Stream.of("List", "Map", "GeoJSON") + .map(typeName -> asList(catalog, null, typeName, null, null, Object.class.getName())) + .collect(toList()); String[] columns = new String[]{"TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SUPERTYPE_CAT", "SUPERTYPE_SCHEM", "SUPERTYPE_NAME"}; int[] sqlTypes = new int[]{VARCHAR, VARCHAR, VARCHAR, VARCHAR, VARCHAR, VARCHAR}; return new ListRecordSet(null, "system", "super_types", - systemColumns(columns, sqlTypes), types); + systemColumns(columns, sqlTypes), typeNames); } @Override @@ -1203,24 +1199,19 @@ public boolean autoCommitFailureClosesAllResultSets() { @Override public ResultSet getClientInfoProperties() { - // TODO: add properties here once implemented String[] columns = new String[]{"NAME", "MAX_LEN", "DEFAULT_VALUE", "DESCRIPTION"}; int[] sqlTypes = new int[]{VARCHAR, INTEGER, VARCHAR, VARCHAR}; - return new ListRecordSet(null, "system", "client_inf_properties", + return new ListRecordSet(null, "system", "client_info_properties", systemColumns(columns, sqlTypes), emptyList()); } @Override public ResultSet getFunctions(String catalog, String schemaPattern, String functionNamePattern) { - //TODO: implement functions support - List> functions = new ArrayList<>(); - String[] columns = new String[]{"FUNCTION_CAT", "FUNCTION_SCHEM", "FUNCTION_NAME", "REMARKS", "FUNCTION_TYPE", "SPECIFIC_NAME"}; - int[] sqlTypes = new int[]{VARCHAR, VARCHAR, VARCHAR, VARCHAR, SMALLINT, VARCHAR}; return new ListRecordSet(null, "system", "functions", - systemColumns(columns, sqlTypes), functions); + systemColumns(columns, sqlTypes), emptyList()); } @Override @@ -1295,8 +1286,8 @@ private String join(String defaultValue, String delimiter, Collection el private int ordinal(ResultSetMetaData md, String columnName) { int ordinal = 0; try { - int n = md.getColumnCount(); - for (int i = 1; i <= n; i++) { + int columnCount = md.getColumnCount(); + for (int i = 1; i <= columnCount; i++) { if (columnName.equals(md.getColumnName(i))) { ordinal = i; break; diff --git a/src/main/java/com/aerospike/jdbc/AerospikeStatement.java b/src/main/java/com/aerospike/jdbc/AerospikeStatement.java index 75b0464..2cf54e4 100644 --- a/src/main/java/com/aerospike/jdbc/AerospikeStatement.java +++ b/src/main/java/com/aerospike/jdbc/AerospikeStatement.java @@ -42,11 +42,7 @@ public class AerospikeStatement implements Statement, SimpleWrapper { public AerospikeStatement(IAerospikeClient client, AerospikeConnection connection) { this.client = client; this.connection = connection; - try { - this.schema = connection.getSchema(); - } catch (SQLException e) { - logger.warning(e.getMessage()); - } + this.schema = connection.getCatalog(); } @Override diff --git a/src/main/java/com/aerospike/jdbc/model/AerospikeSecondaryIndex.java b/src/main/java/com/aerospike/jdbc/model/AerospikeSecondaryIndex.java index 79af678..0afd14a 100644 --- a/src/main/java/com/aerospike/jdbc/model/AerospikeSecondaryIndex.java +++ b/src/main/java/com/aerospike/jdbc/model/AerospikeSecondaryIndex.java @@ -84,17 +84,16 @@ public String getSet() { return set; } - public Integer getBinValuesRatio() - { + public Integer getBinValuesRatio() { return binValuesRatio; } public String toKey() { - return key(namespace, set, indexName); + return key(namespace, set, binName); } - public static String key(String namespace, String set, String indexName) { - return String.format("%s/%s/%s", namespace, set, indexName); + public static String key(String namespace, String set, String binName) { + return String.format("%s.%s.%s", namespace, set, binName); } @SuppressWarnings("all") @@ -149,7 +148,7 @@ public int hashCode() { @Override public String toString() { - return getClass().getSimpleName() + "(" + binName + ", " + indexName + ", " + indexType + - ", " + namespace + ", " + set + ")"; + return String.format("%s(%s, %s, %s, %s, %s)", getClass().getSimpleName(), + binName, indexName, indexType, namespace, set); } } diff --git a/src/main/java/com/aerospike/jdbc/query/SelectQueryHandler.java b/src/main/java/com/aerospike/jdbc/query/SelectQueryHandler.java index add4efe..4262efe 100644 --- a/src/main/java/com/aerospike/jdbc/query/SelectQueryHandler.java +++ b/src/main/java/com/aerospike/jdbc/query/SelectQueryHandler.java @@ -24,7 +24,12 @@ import java.sql.SQLException; import java.sql.Statement; import java.sql.Types; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -130,23 +135,17 @@ private Pair executeQuery(AerospikeQuery query, private Optional secondaryIndex(AerospikeQuery query) { if (aerospikeVersion.isSIndexSupported() && query.isIndexable()) { - Map indexMap = databaseMetadata.getSecondaryIndexes(); + Collection indexes = databaseMetadata.getSecondaryIndexes(query.getSchema()); List binNames = query.getPredicate().getBinNames(); - if (!binNames.isEmpty() && indexMap != null && !indexMap.isEmpty()) { - if (binNames.size() == 1) { - String binName = binNames.get(0); - for (AerospikeSecondaryIndex index : indexMap.values()) { - if (index.getBinName().equals(binName)) { - return Optional.of(index); - } - } - } else { - List indexList = new ArrayList<>(indexMap.values()); - sortIndexList(indexList); - for (AerospikeSecondaryIndex index : indexList) { - if (binNames.contains(index.getBinName())) { - return Optional.of(index); - } + if (!binNames.isEmpty() && indexes != null && !indexes.isEmpty()) { + List indexList = indexes.stream() + .filter(i -> i.getSet().equals(query.getTable())) + .sorted(secondaryIndexComparator()) + .collect(Collectors.toList()); + + for (AerospikeSecondaryIndex index : indexList) { + if (binNames.contains(index.getBinName())) { + return Optional.of(index); } } } @@ -159,12 +158,11 @@ private Pair queryResult(RecordSet recordSet, AerospikeQuery query.getTable(), filterColumns(query)), -1); } - private void sortIndexList(List indexList) { + private Comparator secondaryIndexComparator() { if (aerospikeVersion.isSIndexCardinalitySupported()) { - indexList.sort(Comparator.comparingInt(AerospikeSecondaryIndex::getBinValuesRatio)); - } else { - indexList.sort(Comparator.comparing(AerospikeSecondaryIndex::getBinName)); + return Comparator.comparingInt(AerospikeSecondaryIndex::getBinValuesRatio); } + return Comparator.comparing(AerospikeSecondaryIndex::getBinName); } private List filterColumns(AerospikeQuery query) { diff --git a/src/main/java/com/aerospike/jdbc/sql/AerospikeResultSetMetaData.java b/src/main/java/com/aerospike/jdbc/sql/AerospikeResultSetMetaData.java index dc7e0d7..065b1a0 100644 --- a/src/main/java/com/aerospike/jdbc/sql/AerospikeResultSetMetaData.java +++ b/src/main/java/com/aerospike/jdbc/sql/AerospikeResultSetMetaData.java @@ -145,7 +145,7 @@ public String getColumnName(int column) throws SQLException { @Override public String getSchemaName(int column) throws SQLException { validateColumn(column); - return schema; + return ""; } @Override @@ -166,7 +166,8 @@ public String getTableName(int column) throws SQLException { @Override public String getCatalogName(int column) throws SQLException { - return getSchemaName(column); // return schema + validateColumn(column); + return schema; } @Override diff --git a/src/test/java/com/aerospike/jdbc/DatabaseMetadataTest.java b/src/test/java/com/aerospike/jdbc/DatabaseMetadataTest.java index dfb6119..f938c02 100644 --- a/src/test/java/com/aerospike/jdbc/DatabaseMetadataTest.java +++ b/src/test/java/com/aerospike/jdbc/DatabaseMetadataTest.java @@ -70,9 +70,9 @@ public void testGetSchemas() throws SQLException { assertTrue(schemas.next()); String schemaName = schemas.getString(1); + String catalogName = schemas.getString(2); assertEquals(schemas.getString("TABLE_SCHEM"), schemaName); - assertEquals(schemas.getString("TABLE_CATALOG"), schemaName); - assertEquals(schemas.getString(2), schemaName); + assertEquals(schemas.getString("TABLE_CATALOG"), catalogName); assertFalse(schemas.next()); TestUtil.closeQuietly(schemas); } diff --git a/src/test/java/com/aerospike/jdbc/PreparedQueriesTest.java b/src/test/java/com/aerospike/jdbc/PreparedQueriesTest.java index 31751c1..e497fb5 100644 --- a/src/test/java/com/aerospike/jdbc/PreparedQueriesTest.java +++ b/src/test/java/com/aerospike/jdbc/PreparedQueriesTest.java @@ -43,7 +43,6 @@ public void setUp() throws SQLException { public void tearDown() throws SQLException { Objects.requireNonNull(connection, "connection is null"); PreparedStatement statement = null; - ResultSet resultSet = null; String query = format("delete from %s", tableName); try { statement = connection.prepareStatement(query); @@ -51,7 +50,6 @@ public void tearDown() throws SQLException { assertFalse(result); } finally { closeQuietly(statement); - closeQuietly(resultSet); } assertTrue(statement.getUpdateCount() > 0); } diff --git a/src/test/java/com/aerospike/jdbc/SimpleQueriesTest.java b/src/test/java/com/aerospike/jdbc/SimpleQueriesTest.java index 18b0380..f9fe289 100644 --- a/src/test/java/com/aerospike/jdbc/SimpleQueriesTest.java +++ b/src/test/java/com/aerospike/jdbc/SimpleQueriesTest.java @@ -43,7 +43,6 @@ public void setUp() throws SQLException { public void tearDown() throws SQLException { Objects.requireNonNull(connection, "connection is null"); Statement statement = null; - ResultSet resultSet = null; String query = format("DELETE FROM %s", tableName); try { statement = connection.createStatement(); @@ -51,7 +50,6 @@ public void tearDown() throws SQLException { assertFalse(result); } finally { closeQuietly(statement); - closeQuietly(resultSet); } assertTrue(statement.getUpdateCount() > 0); }