From 2d781e1e5b243e9c3c2fda81d8b6db52f3d3c012 Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Thu, 31 Jul 2025 14:05:22 +0000 Subject: [PATCH 1/2] Empty commit [skip ci] From 849df383045e55e52ea65df4ab2d36e9d0181f4f Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Thu, 31 Jul 2025 23:05:04 +0900 Subject: [PATCH 2/2] Refactor JDBC adapter related code (#2914) --- ...JdbcSchemaLoaderImportIntegrationTest.java | 3 +- .../scalar/db/storage/jdbc/RdbEngineDb2.java | 6 +- .../db/storage/jdbc/RdbEngineMysql.java | 2 +- .../db/storage/jdbc/RdbEngineOracle.java | 6 +- .../db/storage/jdbc/RdbEnginePostgresql.java | 2 +- .../db/storage/jdbc/RdbEngineSqlServer.java | 4 +- .../db/storage/jdbc/RdbEngineSqlite.java | 2 +- .../db/storage/jdbc/RdbEngineStrategy.java | 2 +- .../db/storage/jdbc/query/MergeIntoQuery.java | 142 ------------------ .../db/storage/jdbc/query/MergeQuery.java | 29 +++- .../db/storage/jdbc/query/SelectQuery.java | 2 +- .../storage/jdbc/query/QueryBuilderTest.java | 18 +-- 12 files changed, 44 insertions(+), 174 deletions(-) delete mode 100644 core/src/main/java/com/scalar/db/storage/jdbc/query/MergeIntoQuery.java diff --git a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java index 469f5c1dcc..49b9718495 100644 --- a/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java +++ b/core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcSchemaLoaderImportIntegrationTest.java @@ -27,8 +27,7 @@ public class JdbcSchemaLoaderImportIntegrationTest extends SchemaLoaderImportInt @Override protected Properties getProperties(String testName) { - Properties properties = new Properties(); - properties.putAll(JdbcEnv.getProperties(testName)); + Properties properties = JdbcEnv.getProperties(testName); JdbcConfig config = new JdbcConfig(new DatabaseConfig(properties)); rdbEngine = RdbEngineFactory.create(config); testUtils = new JdbcAdminImportTestUtils(properties); diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java index 43a9c97887..3e8e7d0ad5 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java @@ -14,7 +14,7 @@ import com.scalar.db.io.TimeColumn; import com.scalar.db.io.TimestampColumn; import com.scalar.db.io.TimestampTZColumn; -import com.scalar.db.storage.jdbc.query.MergeIntoQuery; +import com.scalar.db.storage.jdbc.query.MergeQuery; import com.scalar.db.storage.jdbc.query.SelectQuery; import com.scalar.db.storage.jdbc.query.SelectWithLimitQuery; import com.scalar.db.storage.jdbc.query.UpsertQuery; @@ -263,13 +263,13 @@ public String dropIndexSql(String schema, String table, String indexName) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithLimitQuery(builder, limit); } @Override public UpsertQuery buildUpsertQuery(UpsertQuery.Builder builder) { - return MergeIntoQuery.createForDb2(builder); + return new MergeQuery(builder, "SYSIBM.DUAL"); } @Override diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java index aa9c7f74a7..a9818c6105 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java @@ -146,7 +146,7 @@ public String enclose(String name) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithLimitQuery(builder, limit); } diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java index bbd23ce2ef..a89050b467 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java @@ -8,7 +8,7 @@ import com.scalar.db.common.CoreError; import com.scalar.db.exception.storage.ExecutionException; import com.scalar.db.io.DataType; -import com.scalar.db.storage.jdbc.query.MergeIntoQuery; +import com.scalar.db.storage.jdbc.query.MergeQuery; import com.scalar.db.storage.jdbc.query.SelectQuery; import com.scalar.db.storage.jdbc.query.SelectWithFetchFirstNRowsOnly; import com.scalar.db.storage.jdbc.query.UpsertQuery; @@ -173,13 +173,13 @@ public String enclose(String name) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithFetchFirstNRowsOnly(builder, limit); } @Override public UpsertQuery buildUpsertQuery(UpsertQuery.Builder builder) { - return MergeIntoQuery.createForOracle(builder); + return new MergeQuery(builder, "DUAL"); } @Override diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java index 01edddc897..1e616a608c 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java @@ -179,7 +179,7 @@ public String enclose(String name) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithLimitQuery(builder, limit); } diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java index b3e2173933..a21f94ec60 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java @@ -157,13 +157,13 @@ public String enclose(String name) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithTop(builder, limit); } @Override public UpsertQuery buildUpsertQuery(UpsertQuery.Builder builder) { - return new MergeQuery(builder); + return new MergeQuery(builder, null, true); } @Override diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java index e9afca46cb..df6533864d 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java @@ -281,7 +281,7 @@ public String encloseFullTableName(String schema, String table) { } @Override - public SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit) { + public SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit) { return new SelectWithLimitQuery(builder, limit); } diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java index 61025d49c8..21959fbc39 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java @@ -129,7 +129,7 @@ default String encloseFullTableName(String schema, String table) { return enclose(schema) + "." + enclose(table); } - SelectQuery buildSelectQuery(SelectQuery.Builder builder, int limit); + SelectQuery buildSelectWithLimitQuery(SelectQuery.Builder builder, int limit); UpsertQuery buildUpsertQuery(UpsertQuery.Builder builder); diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeIntoQuery.java b/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeIntoQuery.java deleted file mode 100644 index e61bc5032d..0000000000 --- a/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeIntoQuery.java +++ /dev/null @@ -1,142 +0,0 @@ -package com.scalar.db.storage.jdbc.query; - -import com.scalar.db.api.TableMetadata; -import com.scalar.db.io.Column; -import com.scalar.db.io.Key; -import com.scalar.db.storage.jdbc.RdbEngineStrategy; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.sql.PreparedStatement; -import java.sql.SQLException; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; -import javax.annotation.concurrent.ThreadSafe; - -@ThreadSafe -public class MergeIntoQuery implements UpsertQuery { - - private final RdbEngineStrategy rdbEngine; - private final String schema; - private final String table; - private final TableMetadata tableMetadata; - private final Key partitionKey; - private final Optional clusteringKey; - private final Map> columns; - private final String dualTableName; - - public static MergeIntoQuery createForOracle(Builder builder) { - return new MergeIntoQuery(builder, "DUAL"); - } - - public static MergeIntoQuery createForDb2(Builder builder) { - return new MergeIntoQuery(builder, "SYSIBM.DUAL"); - } - - @SuppressFBWarnings("EI_EXPOSE_REP2") - private MergeIntoQuery(Builder builder, String dualTableName) { - rdbEngine = builder.rdbEngine; - schema = builder.schema; - table = builder.table; - tableMetadata = builder.tableMetadata; - partitionKey = builder.partitionKey; - clusteringKey = builder.clusteringKey; - columns = builder.columns; - this.dualTableName = dualTableName; - } - - @Override - public String sql() { - List enclosedKeyNames = new ArrayList<>(); - partitionKey.getColumns().forEach(v -> enclosedKeyNames.add(rdbEngine.enclose(v.getName()))); - clusteringKey.ifPresent( - k -> k.getColumns().forEach(v -> enclosedKeyNames.add(rdbEngine.enclose(v.getName())))); - - List enclosedValueNames = - columns.keySet().stream().map(rdbEngine::enclose).collect(Collectors.toList()); - - StringBuilder sql = new StringBuilder(); - sql.append("MERGE INTO ") - .append(rdbEngine.encloseFullTableName(schema, table)) - .append(" t1 USING (SELECT ") - .append(makeUsingSelectSqlString(enclosedKeyNames)) - .append(" FROM ") - .append(dualTableName) - .append(") t2 ON (") - .append(makePrimaryKeyConditionsSqlString(enclosedKeyNames)) - .append(")"); - if (!columns.isEmpty()) { - sql.append(" WHEN MATCHED THEN UPDATE SET ") - .append(makeUpdateSetSqlString(enclosedValueNames)); - } - sql.append(" WHEN NOT MATCHED THEN INSERT ") - .append(makeInsertSqlString(enclosedKeyNames, enclosedValueNames)); - return sql.toString(); - } - - private String makeUsingSelectSqlString(List enclosedKeyNames) { - return enclosedKeyNames.stream().map(n -> "? " + n).collect(Collectors.joining(",")); - } - - private String makePrimaryKeyConditionsSqlString(List enclosedKeyNames) { - return enclosedKeyNames.stream() - .map(n -> "t1." + n + "=t2." + n) - .collect(Collectors.joining(" AND ")); - } - - private String makeUpdateSetSqlString(List enclosedValueNames) { - return enclosedValueNames.stream().map(n -> n + "=?").collect(Collectors.joining(",")); - } - - private String makeInsertSqlString( - List enclosedKeyNames, List enclosedValueNames) { - List names = new ArrayList<>(enclosedKeyNames); - names.addAll(enclosedValueNames); - return "(" - + String.join(",", names) - + ") VALUES (" - + names.stream().map(n -> "?").collect(Collectors.joining(",")) - + ")"; - } - - @Override - public void bind(PreparedStatement preparedStatement) throws SQLException { - PreparedStatementBinder binder = - new PreparedStatementBinder(preparedStatement, tableMetadata, rdbEngine); - - // For the USING SELECT statement - for (Column column : partitionKey.getColumns()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - if (clusteringKey.isPresent()) { - for (Column column : clusteringKey.get().getColumns()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - } - - // For the UPDATE statement - for (Column column : columns.values()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - - // For the INSERT statement - for (Column column : partitionKey.getColumns()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - if (clusteringKey.isPresent()) { - for (Column column : clusteringKey.get().getColumns()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - } - for (Column column : columns.values()) { - column.accept(binder); - binder.throwSQLExceptionIfOccurred(); - } - } -} diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeQuery.java b/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeQuery.java index 24eefe38bc..1ffa217bf3 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeQuery.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/query/MergeQuery.java @@ -12,6 +12,7 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; @ThreadSafe @@ -24,9 +25,16 @@ public class MergeQuery implements UpsertQuery { private final Key partitionKey; private final Optional clusteringKey; private final Map> columns; + @Nullable private final String dualTableName; + private final boolean semicolonAdded; @SuppressFBWarnings("EI_EXPOSE_REP2") - public MergeQuery(Builder builder) { + public MergeQuery(Builder builder, @Nullable String dualTableName) { + this(builder, dualTableName, false); + } + + @SuppressFBWarnings("EI_EXPOSE_REP2") + public MergeQuery(Builder builder, @Nullable String dualTableName, boolean semicolonAdded) { rdbEngine = builder.rdbEngine; schema = builder.schema; table = builder.table; @@ -34,6 +42,8 @@ public MergeQuery(Builder builder) { partitionKey = builder.partitionKey; clusteringKey = builder.clusteringKey; columns = builder.columns; + this.dualTableName = dualTableName; + this.semicolonAdded = semicolonAdded; } @Override @@ -47,20 +57,23 @@ public String sql() { columns.keySet().stream().map(rdbEngine::enclose).collect(Collectors.toList()); StringBuilder sql = new StringBuilder(); - sql.append("MERGE ") + sql.append("MERGE INTO ") .append(rdbEngine.encloseFullTableName(schema, table)) .append(" t1 USING (SELECT ") - .append(makeUsingSelectSqlString(enclosedKeyNames)) - .append(") t2 ON (") - .append(makePrimaryKeyConditionsSqlString(enclosedKeyNames)) - .append(")"); + .append(makeUsingSelectSqlString(enclosedKeyNames)); + if (dualTableName != null) { + sql.append(" FROM ").append(dualTableName); + } + sql.append(") t2 ON (").append(makePrimaryKeyConditionsSqlString(enclosedKeyNames)).append(")"); if (!columns.isEmpty()) { sql.append(" WHEN MATCHED THEN UPDATE SET ") .append(makeUpdateSetSqlString(enclosedValueNames)); } sql.append(" WHEN NOT MATCHED THEN INSERT ") - .append(makeInsertSqlString(enclosedKeyNames, enclosedValueNames)) - .append(";"); + .append(makeInsertSqlString(enclosedKeyNames, enclosedValueNames)); + if (semicolonAdded) { + sql.append(";"); + } return sql.toString(); } diff --git a/core/src/main/java/com/scalar/db/storage/jdbc/query/SelectQuery.java b/core/src/main/java/com/scalar/db/storage/jdbc/query/SelectQuery.java index 5cc5834264..43ca382930 100644 --- a/core/src/main/java/com/scalar/db/storage/jdbc/query/SelectQuery.java +++ b/core/src/main/java/com/scalar/db/storage/jdbc/query/SelectQuery.java @@ -152,7 +152,7 @@ public Builder limit(int limit) { public SelectQuery build() { if (limit > 0) { - return rdbEngine.buildSelectQuery(this, limit); + return rdbEngine.buildSelectWithLimitQuery(this, limit); } return new SimpleSelectQuery(this); } diff --git a/core/src/test/java/com/scalar/db/storage/jdbc/query/QueryBuilderTest.java b/core/src/test/java/com/scalar/db/storage/jdbc/query/QueryBuilderTest.java index 682e189885..69d46a70d4 100644 --- a/core/src/test/java/com/scalar/db/storage/jdbc/query/QueryBuilderTest.java +++ b/core/src/test/java/com/scalar/db/storage/jdbc/query/QueryBuilderTest.java @@ -1292,7 +1292,7 @@ public void upsertQueryTest(RdbEngine rdbEngineType) throws SQLException { RdbEngineStrategy rdbEngine = RdbEngine.createRdbEngineStrategy(rdbEngineType); QueryBuilder queryBuilder = new QueryBuilder(rdbEngine); - String expectedQuery = ""; + String expectedQuery; UpsertQuery query; PreparedStatement preparedStatement; @@ -1329,7 +1329,7 @@ public void upsertQueryTest(RdbEngine rdbEngineType) throws SQLException { break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1) t2 ON (t1.p1=t2.p1) " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1) t2 ON (t1.p1=t2.p1) " + "WHEN MATCHED THEN UPDATE SET v1=?,v2=?,v3=? " + "WHEN NOT MATCHED THEN INSERT (p1,v1,v2,v3) VALUES (?,?,?,?);"; break; @@ -1408,7 +1408,7 @@ public void upsertQueryTest(RdbEngine rdbEngineType) throws SQLException { break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1,? c1) t2 " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1,? c1) t2 " + "ON (t1.p1=t2.p1 AND t1.c1=t2.c1) " + "WHEN MATCHED THEN UPDATE SET v1=?,v2=?,v3=? " + "WHEN NOT MATCHED THEN INSERT (p1,c1,v1,v2,v3) VALUES (?,?,?,?,?);"; @@ -1494,7 +1494,7 @@ public void upsertQueryTest(RdbEngine rdbEngineType) throws SQLException { break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "ON (t1.p1=t2.p1 AND t1.p2=t2.p2 AND t1.c1=t2.c1 AND t1.c2=t2.c2) " + "WHEN MATCHED THEN UPDATE SET v1=?,v2=?,v3=?,v4=? " + "WHEN NOT MATCHED THEN INSERT (p1,p2,c1,c2,v1,v2,v3,v4) VALUES (?,?,?,?,?,?,?,?);"; @@ -1593,7 +1593,7 @@ public void upsertQueryTest(RdbEngine rdbEngineType) throws SQLException { break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "ON (t1.p1=t2.p1 AND t1.p2=t2.p2 AND t1.c1=t2.c1 AND t1.c2=t2.c2) " + "WHEN MATCHED THEN UPDATE SET v1=?,v2=?,v3=?,v4=?,v5=? " + "WHEN NOT MATCHED THEN INSERT (p1,p2,c1,c2,v1,v2,v3,v4,v5) " @@ -1671,7 +1671,7 @@ public void upsertQueryWithoutValuesTest(RdbEngine rdbEngineType) throws SQLExce RdbEngineStrategy rdbEngine = RdbEngine.createRdbEngineStrategy(rdbEngineType); QueryBuilder queryBuilder = new QueryBuilder(rdbEngine); - String expectedQuery = ""; + String expectedQuery; UpsertQuery query; PreparedStatement preparedStatement; @@ -1697,7 +1697,7 @@ public void upsertQueryWithoutValuesTest(RdbEngine rdbEngineType) throws SQLExce break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1) t2 ON (t1.p1=t2.p1) " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1) t2 ON (t1.p1=t2.p1) " + "WHEN NOT MATCHED THEN INSERT (p1) VALUES (?);"; break; case SQLITE: @@ -1755,7 +1755,7 @@ public void upsertQueryWithoutValuesTest(RdbEngine rdbEngineType) throws SQLExce break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1,? c1) t2 " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1,? c1) t2 " + "ON (t1.p1=t2.p1 AND t1.c1=t2.c1) " + "WHEN NOT MATCHED THEN INSERT (p1,c1) VALUES (?,?);"; break; @@ -1822,7 +1822,7 @@ public void upsertQueryWithoutValuesTest(RdbEngine rdbEngineType) throws SQLExce break; case SQL_SERVER: expectedQuery = - "MERGE n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "MERGE INTO n1.t1 t1 USING (SELECT ? p1,? p2,? c1,? c2) t2 " + "ON (t1.p1=t2.p1 AND t1.p2=t2.p2 AND t1.c1=t2.c1 AND t1.c2=t2.c2) " + "WHEN NOT MATCHED THEN INSERT (p1,p2,c1,c2) VALUES (?,?,?,?);"; break;