From ee66ddcf0af08ec923cb510e0ad0c8e0a7893d5e Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 30 Apr 2024 10:48:47 +0800 Subject: [PATCH 1/4] Fix jdbc list table bugs. --- .../jdbc/operation/JdbcTableOperations.java | 10 +++-- .../jdbc/operation/SqliteTableOperations.java | 2 +- .../doris/operation/DorisTableOperations.java | 2 +- .../mysql/operation/MysqlTableOperations.java | 13 ++++-- .../test/MysqlTableOperationsIT.java | 45 +++++++++++++++++++ .../operation/PostgreSqlTableOperations.java | 2 +- server/src/main/resources/project.properties | 7 +++ 7 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 server/src/main/resources/project.properties diff --git a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java index 1581189e5ac..8494fac65af 100644 --- a/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java +++ b/catalogs/catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/operation/JdbcTableOperations.java @@ -23,6 +23,7 @@ import com.datastrato.gravitino.rel.expressions.transforms.Transform; import com.datastrato.gravitino.rel.indexes.Index; import com.datastrato.gravitino.rel.indexes.Indexes; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import java.sql.Connection; import java.sql.DatabaseMetaData; @@ -110,7 +111,7 @@ public void drop(String databaseName, String tableName) throws NoSuchTableExcept public List listTables(String databaseName) throws NoSuchSchemaException { try (Connection connection = getConnection(databaseName)) { final List names = Lists.newArrayList(); - try (ResultSet tables = getTables(connection)) { + try (ResultSet tables = getTables(connection, databaseName)) { while (tables.next()) { if (Objects.equals(tables.getString("TABLE_SCHEM"), databaseName)) { names.add(tables.getString("TABLE_NAME")); @@ -259,9 +260,9 @@ public void purge(String databaseName, String tableName) throws NoSuchTableExcep } } - protected ResultSet getTables(Connection connection) throws SQLException { + @VisibleForTesting + public ResultSet getTables(Connection connection, String databaseName) throws SQLException { final DatabaseMetaData metaData = connection.getMetaData(); - String databaseName = connection.getSchema(); return metaData.getTables(databaseName, databaseName, null, JdbcConnectorUtils.getTableTypes()); } @@ -415,7 +416,8 @@ protected JdbcColumn getJdbcColumnFromTable(JdbcTable jdbcTable, String colName) "Column %s does not exist in table %s", colName, jdbcTable.name())); } - protected Connection getConnection(String catalog) throws SQLException { + @VisibleForTesting + public Connection getConnection(String catalog) throws SQLException { Connection connection = dataSource.getConnection(); connection.setCatalog(catalog); return connection; diff --git a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java index 66fa734ce08..ede45b83547 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java +++ b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/SqliteTableOperations.java @@ -109,7 +109,7 @@ protected boolean getAutoIncrementInfo(ResultSet columns) { public List listTables(String databaseName) throws NoSuchSchemaException { try (Connection connection = getConnection(databaseName)) { final List names = Lists.newArrayList(); - try (ResultSet tables = getTables(connection)) { + try (ResultSet tables = getTables(connection, databaseName)) { // tables.getString("TABLE_SCHEM") is always null. while (tables.next()) { names.add(tables.getString("TABLE_NAME")); diff --git a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java index afa168c33e5..f768c60bcce 100644 --- a/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java +++ b/catalogs/catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/operation/DorisTableOperations.java @@ -52,7 +52,7 @@ public List listTables(String databaseName) throws NoSuchSchemaException final List names = Lists.newArrayList(); try (Connection connection = getConnection(databaseName); - ResultSet tables = getTables(connection)) { + ResultSet tables = getTables(connection, databaseName)) { while (tables.next()) { if (Objects.equals(tables.getString("TABLE_CAT"), databaseName)) { names.add(tables.getString("TABLE_NAME")); diff --git a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java index 0446713c77b..9cd977caba4 100644 --- a/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java +++ b/catalogs/catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java @@ -58,13 +58,20 @@ public List listTables(String databaseName) throws NoSuchSchemaException final List names = Lists.newArrayList(); try (Connection connection = getConnection(databaseName); - ResultSet tables = getTables(connection)) { + ResultSet tables = getTables(connection, databaseName)) { + long total = 0; while (tables.next()) { - if (Objects.equals(tables.getString("TABLE_CAT"), databaseName)) { + String dbNameFromResult = tables.getString("TABLE_CAT"); + if (Objects.equals(dbNameFromResult, databaseName)) { names.add(tables.getString("TABLE_NAME")); } + total++; } - LOG.info("Finished listing tables size {} for database name {} ", names.size(), databaseName); + LOG.info( + "Finished listing tables size {} for database name {}, total scan = {}", + names.size(), + databaseName, + total); return names; } catch (final SQLException se) { throw this.exceptionMapper.toGravitinoException(se); diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java index fd4cf52fa31..935ad3f32fb 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java @@ -21,6 +21,8 @@ import com.datastrato.gravitino.rel.types.Type; import com.datastrato.gravitino.rel.types.Types; import com.datastrato.gravitino.utils.RandomNameUtils; +import java.sql.Connection; +import java.sql.ResultSet; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Arrays; @@ -771,6 +773,49 @@ public void testCreateMultipleTables() { Assertions.assertFalse(tables.contains(test_table_2)); } + @Test + void testListTable() { + String[] dbNames = new String[] {"db1", "db2"}; + String[] tableNames = new String[] {"table1", "table2", "table3"}; + for (String dbName : dbNames) { + DATABASE_OPERATIONS.create(dbName, null, null); + } + + for (String dbName : dbNames) { + for (String tableName : tableNames) { + TABLE_OPERATIONS.create( + dbName, + tableName, + new JdbcColumn[] { + JdbcColumn.builder() + .withName("col_1") + .withType(Types.DecimalType.of(10, 2)) + .withComment("test_decimal") + .withNullable(false) + .build() + }, + "test_comment", + null, + null, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + } + } + + for (String dbName : dbNames) { + try (Connection connection = TABLE_OPERATIONS.getConnection(dbName); + ResultSet resultSet = TABLE_OPERATIONS.getTables(connection, dbName)) { + List tables = new ArrayList<>(); + while (resultSet.next()) { + tables.add(resultSet.getString("TABLE_NAME")); + } + Assertions.assertEquals(Arrays.asList(tableNames), tables); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + } + @Test public void testLoadTableDefaultProperties() { String test_table_1 = RandomNameUtils.genRandomName("properties_table_"); diff --git a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java index 3d5fc2b9a25..8b09c263573 100644 --- a/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java +++ b/catalogs/catalog-jdbc-postgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java @@ -675,7 +675,7 @@ protected ResultSet getPrimaryKeys(String schemaName, String tableName, Database } @Override - protected Connection getConnection(String schema) throws SQLException { + public Connection getConnection(String schema) throws SQLException { Connection connection = dataSource.getConnection(); connection.setCatalog(database); connection.setSchema(schema); diff --git a/server/src/main/resources/project.properties b/server/src/main/resources/project.properties new file mode 100644 index 00000000000..2b843539c89 --- /dev/null +++ b/server/src/main/resources/project.properties @@ -0,0 +1,7 @@ +# +# Copyright 2023 Datastrato Pvt Ltd. +# This software is licensed under the Apache License version 2. +# +project.version=0.5.0-SNAPSHOT +compile.date=23/04/2024 17:39:38 +git.commit.id=ebdf514eee9fe450ef79dcb1bf22c4454f75b40a From 31f256063a15651728151fa6922b406d7297b56f Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 30 Apr 2024 10:54:39 +0800 Subject: [PATCH 2/4] Remove --- server/src/main/resources/project.properties | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 server/src/main/resources/project.properties diff --git a/server/src/main/resources/project.properties b/server/src/main/resources/project.properties deleted file mode 100644 index 2b843539c89..00000000000 --- a/server/src/main/resources/project.properties +++ /dev/null @@ -1,7 +0,0 @@ -# -# Copyright 2023 Datastrato Pvt Ltd. -# This software is licensed under the Apache License version 2. -# -project.version=0.5.0-SNAPSHOT -compile.date=23/04/2024 17:39:38 -git.commit.id=ebdf514eee9fe450ef79dcb1bf22c4454f75b40a From fa26d295a5fe96c63878a70f4cef89719859dec2 Mon Sep 17 00:00:00 2001 From: yuqi Date: Tue, 30 Apr 2024 15:41:56 +0800 Subject: [PATCH 3/4] Add PG list table test. --- .../test/MysqlTableOperationsIT.java | 2 +- .../test/PostgreSqlTableOperationsIT.java | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java index 935ad3f32fb..9445ea90662 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java @@ -774,7 +774,7 @@ public void testCreateMultipleTables() { } @Test - void testListTable() { + void testListMySQLTable() { String[] dbNames = new String[] {"db1", "db2"}; String[] tableNames = new String[] {"table1", "table2", "table3"}; for (String dbName : dbNames) { diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java index 65e0e890020..2ffc3757192 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java @@ -24,8 +24,10 @@ import com.datastrato.gravitino.rel.types.Types; import com.datastrato.gravitino.utils.RandomNameUtils; import java.sql.Connection; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -622,4 +624,47 @@ public void testCreateIndexTable() { gravitinoRuntimeException.getMessage(), "column \"no_exist_1\" named in key does not exist")); } + + @Test + void testListPostgreSQLTable() { + String[] dbNames = new String[] {"db1", "db2"}; + String[] tableNames = new String[] {"table1", "table2", "table3"}; + for (String dbName : dbNames) { + DATABASE_OPERATIONS.create(dbName, null, null); + } + + for (String dbName : dbNames) { + for (String tableName : tableNames) { + TABLE_OPERATIONS.create( + dbName, + tableName, + new JdbcColumn[] { + JdbcColumn.builder() + .withName("col_1") + .withType(Types.DecimalType.of(10, 2)) + .withComment("test_decimal") + .withNullable(false) + .build() + }, + "test_comment", + null, + null, + Distributions.NONE, + Indexes.EMPTY_INDEXES); + } + } + + for (String dbName : dbNames) { + try (Connection connection = TABLE_OPERATIONS.getConnection(dbName); + ResultSet resultSet = TABLE_OPERATIONS.getTables(connection, dbName)) { + List tables = new ArrayList<>(); + while (resultSet.next()) { + tables.add(resultSet.getString("TABLE_NAME")); + } + Assertions.assertEquals(Arrays.asList(tableNames), tables); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + } } From 70c8fcf6fc5e2c8b8b49056b16c6f16d046b8848 Mon Sep 17 00:00:00 2001 From: yuqi Date: Mon, 6 May 2024 20:17:43 +0800 Subject: [PATCH 4/4] Optimize test. --- .../mysql/integration/test/MysqlTableOperationsIT.java | 5 ++--- .../integration/test/PostgreSqlTableOperationsIT.java | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java index 9445ea90662..4ad6eb37d02 100644 --- a/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java +++ b/catalogs/catalog-jdbc-mysql/src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/MysqlTableOperationsIT.java @@ -23,6 +23,7 @@ import com.datastrato.gravitino.utils.RandomNameUtils; import java.sql.Connection; import java.sql.ResultSet; +import java.sql.SQLException; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Arrays; @@ -774,7 +775,7 @@ public void testCreateMultipleTables() { } @Test - void testListMySQLTable() { + void testListMySQLTable() throws SQLException { String[] dbNames = new String[] {"db1", "db2"}; String[] tableNames = new String[] {"table1", "table2", "table3"}; for (String dbName : dbNames) { @@ -810,8 +811,6 @@ void testListMySQLTable() { tables.add(resultSet.getString("TABLE_NAME")); } Assertions.assertEquals(Arrays.asList(tableNames), tables); - } catch (Exception e) { - throw new RuntimeException(e); } } } diff --git a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java index 2ffc3757192..c3835e3e79a 100644 --- a/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java +++ b/catalogs/catalog-jdbc-postgresql/src/test/java/com/datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java @@ -626,7 +626,7 @@ public void testCreateIndexTable() { } @Test - void testListPostgreSQLTable() { + void testListPostgreSQLTable() throws SQLException { String[] dbNames = new String[] {"db1", "db2"}; String[] tableNames = new String[] {"table1", "table2", "table3"}; for (String dbName : dbNames) { @@ -662,8 +662,6 @@ void testListPostgreSQLTable() { tables.add(resultSet.getString("TABLE_NAME")); } Assertions.assertEquals(Arrays.asList(tableNames), tables); - } catch (Exception e) { - throw new RuntimeException(e); } } }