From ad86fab6ed5084e2039d0703e8533875bcb3c5f9 Mon Sep 17 00:00:00 2001 From: Zhou Kang Date: Tue, 19 Mar 2024 20:02:39 +0800 Subject: [PATCH] fix PR reviews --- .../jdbc/operation/SqliteTableOperations.java | 3 +-- .../operation/TestJdbcTableOperations.java | 3 ++- .../mysql/operation/MysqlTableOperations.java | 3 +-- .../test/MysqlTableOperationsIT.java | 27 ++++++++++--------- .../operation/PostgreSqlTableOperations.java | 3 +-- .../test/PostgreSqlTableOperationsIT.java | 17 ++++++------ 6 files changed, 28 insertions(+), 28 deletions(-) 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 4e7301ba878..66fa734ce08 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 @@ -33,8 +33,7 @@ protected String generateCreateTableSql( Distribution distribution, Index[] indexes) { Preconditions.checkArgument( - null == distribution || distribution == Distributions.NONE, - "Currently we do not support distribution in Sqlite."); + distribution == Distributions.NONE, "SQLite does not support distribution"); StringBuilder sqlBuilder = new StringBuilder(); sqlBuilder.append("CREATE TABLE ").append(tableName).append(" ("); diff --git a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java index 1baf6de0323..9ac34198bee 100644 --- a/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java +++ b/catalogs/catalog-jdbc-common/src/test/java/com/datastrato/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java @@ -16,6 +16,7 @@ import com.datastrato.gravitino.exceptions.NoSuchTableException; import com.datastrato.gravitino.rel.Column; import com.datastrato.gravitino.rel.TableChange; +import com.datastrato.gravitino.rel.expressions.distributions.Distributions; import com.datastrato.gravitino.rel.indexes.Indexes; import com.datastrato.gravitino.rel.types.Type; import com.datastrato.gravitino.rel.types.Types; @@ -140,7 +141,7 @@ public void testOperationTable() { null, properties, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES)); // list table. 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 0b515b9501d..2ad0f3890d2 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 @@ -87,8 +87,7 @@ protected String generateCreateTableSql( } Preconditions.checkArgument( - null == distribution || distribution == Distributions.NONE, - "Currently we do not support distribution in mysql"); + distribution == Distributions.NONE, "MySQL does not support distribution"); validateIncrementCol(columns, indexes); StringBuilder sqlBuilder = new StringBuilder(); 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 2b2fe25aa55..f60a81464a5 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 @@ -13,6 +13,7 @@ import com.datastrato.gravitino.exceptions.NoSuchTableException; import com.datastrato.gravitino.rel.Column; import com.datastrato.gravitino.rel.TableChange; +import com.datastrato.gravitino.rel.expressions.distributions.Distributions; import com.datastrato.gravitino.rel.expressions.literals.Literals; import com.datastrato.gravitino.rel.indexes.Index; import com.datastrato.gravitino.rel.indexes.Indexes; @@ -86,7 +87,7 @@ public void testOperationTable() { tableComment, properties, null, - null, + Distributions.NONE, indexes); // list table @@ -219,7 +220,7 @@ public void testAlterTable() { tableComment, properties, null, - null, + Distributions.NONE, indexes); JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); assertionsTableInfo(tableName, tableComment, columns, properties, indexes, load); @@ -462,7 +463,7 @@ public void testCreateAndLoadTable() { tableComment, properties, null, - null, + Distributions.NONE, indexes); JdbcTable loaded = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); @@ -561,7 +562,7 @@ public void testCreateAllTypeTable() { tableComment, Collections.emptyMap(), null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); @@ -605,7 +606,7 @@ public void testCreateNotSupportTypeTable() { tableComment, emptyMap, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); }); Assertions.assertTrue( @@ -635,7 +636,7 @@ public void testCreateMultipleTables() { "test_comment", null, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); String testDb = "test_db_2"; @@ -660,7 +661,7 @@ public void testCreateMultipleTables() { "test_comment", null, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); tables = TABLE_OPERATIONS.listTables(TEST_DB_NAME); @@ -684,7 +685,7 @@ public void testLoadTableDefaultProperties() { "test_comment", null, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, test_table_1); Assertions.assertEquals("InnoDB", load.properties().get(MYSQL_ENGINE_KEY)); @@ -728,7 +729,7 @@ public void testAutoIncrement() { Indexes.unique("uk_1", new String[][] {{"col_1"}}) }; TABLE_OPERATIONS.create( - TEST_DB_NAME, tableName, columns, comment, properties, null, null, indexes); + TEST_DB_NAME, tableName, columns, comment, properties, null, Distributions.NONE, indexes); JdbcTable table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); assertionsTableInfo( @@ -747,7 +748,7 @@ public void testAutoIncrement() { Indexes.unique("uk_2", new String[][] {{"col_2"}}) }; TABLE_OPERATIONS.create( - TEST_DB_NAME, tableName, columns, comment, properties, null, null, indexes); + TEST_DB_NAME, tableName, columns, comment, properties, null, Distributions.NONE, indexes); table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); assertionsTableInfo( @@ -762,7 +763,7 @@ public void testAutoIncrement() { // Test create increment key for col_1 + col_3 uk. indexes = new Index[] {Indexes.unique("uk_2_3", new String[][] {{"col_1"}, {"col_3"}})}; TABLE_OPERATIONS.create( - TEST_DB_NAME, tableName, columns, comment, properties, null, null, indexes); + TEST_DB_NAME, tableName, columns, comment, properties, null, Distributions.NONE, indexes); table = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); assertionsTableInfo( @@ -786,7 +787,7 @@ public void testAutoIncrement() { comment, properties, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES)); Assertions.assertTrue( StringUtils.contains( @@ -820,7 +821,7 @@ public void testAutoIncrement() { comment, properties, null, - null, + Distributions.NONE, primaryIndex)); Assertions.assertTrue( StringUtils.contains( 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 5d709641a35..55ce07ea3d9 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 @@ -83,8 +83,7 @@ protected String generateCreateTableSql( "Currently we do not support Partitioning in PostgreSQL"); } Preconditions.checkArgument( - null == distribution || distribution == Distributions.NONE, - "Currently we do not support distribution in PostgreSQL"); + distribution == Distributions.NONE, "PostgreSQL does not support distribution"); StringBuilder sqlBuilder = new StringBuilder(); sqlBuilder 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 921423f477f..c03fb70667f 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 @@ -16,6 +16,7 @@ import com.datastrato.gravitino.exceptions.GravitinoRuntimeException; import com.datastrato.gravitino.exceptions.NoSuchTableException; import com.datastrato.gravitino.rel.TableChange; +import com.datastrato.gravitino.rel.expressions.distributions.Distributions; import com.datastrato.gravitino.rel.indexes.Index; import com.datastrato.gravitino.rel.indexes.Indexes; import com.datastrato.gravitino.rel.types.Type; @@ -80,7 +81,7 @@ public void testOperationTable() { tableComment, properties, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); // list table @@ -307,7 +308,7 @@ public void testCreateAllTypeTable() { tableComment, Collections.emptyMap(), null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); @@ -362,7 +363,7 @@ public void testCreateMultipleTable() throws SQLException { null, null, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); List tableNames = TABLE_OPERATIONS.listTables(TEST_DB_NAME); @@ -391,7 +392,7 @@ public void testCreateMultipleTable() throws SQLException { null, null, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); tableNames = postgreSqlTableOperations.listTables(TEST_DB_NAME); Assertions.assertFalse(tableNames.contains(table_2)); @@ -443,7 +444,7 @@ public void testCreateAutoIncrementTable() { tableComment, properties, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); // list table @@ -485,7 +486,7 @@ public void testCreateAutoIncrementTable() { tableComment, properties, null, - null, + Distributions.NONE, Indexes.EMPTY_INDEXES); }); @@ -544,7 +545,7 @@ public void testCreateIndexTable() { tableComment, properties, null, - null, + Distributions.NONE, indexes); JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName); @@ -570,7 +571,7 @@ public void testCreateIndexTable() { tableComment, properties, null, - null, + Distributions.NONE, primaryIndex); }); Assertions.assertTrue(