Browse Source

Update tests and optimize conflict strategy.

still need to figure out mssql idenity insert alternative and mysql update when insert matches conflict.
Christoph Strobl 1 week ago
parent
commit
c62097e8cc
No known key found for this signature in database
GPG Key ID: E6054036D0C37A4B
  1. 5
      spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/DialectResolver.java
  2. 112
      spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java
  3. 16
      spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/sql/render/UpsertRendererUnitTests.java
  4. 82
      spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OracleUpsertRenderContext.java
  5. 10
      spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/PostgresUpsertRenderContext.java
  6. 45
      spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/StandardSqlUpsertRenderContext.java
  7. 59
      spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/UpsertRenderContextUnitTests.java

5
spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/DialectResolver.java

@ -205,6 +205,11 @@ public class DialectResolver { @@ -205,6 +205,11 @@ public class DialectResolver {
this.arrayColumns = new JdbcArrayColumnsAdapter(delegate.getArraySupport());
}
@Override
public String getName() {
return delegate.getName();
}
@Override
public LimitClause limit() {
return delegate.limit();

112
spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java

@ -189,34 +189,108 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests { @@ -189,34 +189,108 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
return entity;
}
private void withSqlServerIdentityInsertOn(JdbcAggregateOperations jdbcAggregateTemplate, String tableName,
Runnable action) {
if (jdbcAggregateTemplate.getDataAccessStrategy().getDialect() instanceof SqlServerDialect) {
jdbc.getJdbcOperations().execute("SET IDENTITY_INSERT " + tableName + " ON");
try {
action.run();
} finally {
jdbc.getJdbcOperations().execute("SET IDENTITY_INSERT " + tableName + " OFF");
}
} else {
action.run();
}
}
@Test // GH-493
void upsertInsertsWhenIdDoesNotExistAndUpdatesWhenItExists() {
assumeThat(template).isInstanceOf(JdbcAggregateTemplate.class);
JdbcAggregateTemplate jdbcTemplate = (JdbcAggregateTemplate) template;
assumeThat(jdbcTemplate.getDataAccessStrategy().getDialect().getUpsertRenderContext()).isNotNull();
withSqlServerIdentityInsertOn(template, "with_insert_only", () -> {
if(template.getDataAccessStrategy().getDialect() instanceof SqlServerDialect) {
String tableName = "with_insert_only";
jdbc.getJdbcOperations().execute("SET IDENTITY_INSERT " + tableName + " ON");
}
WithInsertOnly entity = new WithInsertOnly();
entity.id = 8888L;
entity.insertOnly = "upserted";
template.upsert(entity);
WithInsertOnly entity = new WithInsertOnly();
entity.id = 8888L;
entity.insertOnly = "upserted";
template.upsert(entity);
assertThat(template.findById(8888L, WithInsertOnly.class).insertOnly).isEqualTo("upserted");
entity.insertOnly = "updated";
template.upsert(entity);
assertThat(template.findById(8888L, WithInsertOnly.class).insertOnly).isEqualTo("upserted");
assertThat(template.findById(8888L, WithInsertOnly.class).insertOnly).isEqualTo("updated");
});
}
entity.insertOnly = "updated";
template.upsert(entity);
@Test // GH-493
void upsertWhenMatchedAndUpdateAssignmentsEqualConflictKeyOnly() {
assertThat(template.findById(8888L, WithInsertOnly.class).insertOnly).isEqualTo("updated");
long id = 8889L;
withSqlServerIdentityInsertOn(template, "with_id_only", () -> {
if(template.getDataAccessStrategy().getDialect() instanceof SqlServerDialect) {
String tableName = "with_insert_only";
jdbc.getJdbcOperations().execute("SET IDENTITY_INSERT " + tableName + " OFF");
}
WithIdOnly first = new WithIdOnly();
first.id = id;
template.upsert(first);
WithIdOnly second = new WithIdOnly();
second.id = id;
template.upsert(second);
assertThat(template.findById(id, WithIdOnly.class).id).isEqualTo(id);
});
}
@Test // GH-493
void upsertNoOpWhenNonKeyColumnsAlreadyMatch() {
long id = 8890L;
withSqlServerIdentityInsertOn(template, "LEGO_SET", () -> {
LegoSet lego = new LegoSet();
lego.id = id;
lego.name = "millennium";
template.upsert(lego);
template.upsert(lego);
assertThat(template.findById(id, LegoSet.class).name).isEqualTo("millennium");
});
}
@Test // GH-493
void upsertAfterDeleteInsertsAgain() {
long id = 8891L;
withSqlServerIdentityInsertOn(template, "LEGO_SET", () -> {
LegoSet lego = new LegoSet();
lego.id = id;
lego.name = "first";
template.upsert(lego);
template.deleteById(id, LegoSet.class);
lego.name = "second";
template.upsert(lego);
assertThat(template.findById(id, LegoSet.class).name).isEqualTo("second");
});
}
@Test // GH-493
void upsertExistingRowWithSameInsertOnlyValue() {
withSqlServerIdentityInsertOn(template, "with_insert_only", () -> {
long id = 8892L;
WithInsertOnly entity = new WithInsertOnly();
entity.id = id;
entity.insertOnly = "unchanged";
template.upsert(entity);
template.upsert(entity);
assertThat(template.findById(id, WithInsertOnly.class).insertOnly).isEqualTo("unchanged");
});
}
@Test // GH-1446

16
spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/sql/render/UpsertRendererUnitTests.java

@ -18,6 +18,7 @@ package org.springframework.data.jdbc.core.sql.render; @@ -18,6 +18,7 @@ package org.springframework.data.jdbc.core.sql.render;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.jupiter.api.Test;
import org.springframework.data.jdbc.core.dialect.JdbcOracleDialect;
import org.springframework.data.relational.core.dialect.RenderContextFactory;
import org.springframework.data.relational.core.sql.SQL;
import org.springframework.data.relational.core.sql.StatementBuilder;
@ -124,4 +125,19 @@ class UpsertRendererUnitTests { @@ -124,4 +125,19 @@ class UpsertRendererUnitTests {
assertThat(sql).contains("WHEN MATCHED THEN UPDATE SET");
assertThat(sql).contains("WHEN NOT MATCHED THEN INSERT");
}
@Test // GH-493
void oracleIdOnlyMergeOmitsWhenMatchedUpdate() {
Table table = SQL.table("ent");
Upsert upsert = StatementBuilder.upsert().table(table).columnValue(table.column("id").set(SQL.bindMarker("id")))
.where(table.column("id").isEqualTo(SQL.bindMarker("id"))).build();
var context = new RenderContextFactory(JdbcOracleDialect.INSTANCE).createRenderContext();
String sql = SqlRenderer.create(context).render(upsert);
assertThat(sql).contains("MERGE INTO");
assertThat(sql).contains("WHEN NOT MATCHED THEN INSERT");
assertThat(sql).doesNotContain("WHEN MATCHED THEN UPDATE SET");
}
}

82
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/OracleUpsertRenderContext.java

@ -16,17 +16,14 @@ @@ -16,17 +16,14 @@
package org.springframework.data.relational.core.sql.render;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import org.springframework.data.relational.core.sql.IdentifierProcessing;
import org.springframework.data.relational.core.sql.SqlIdentifier;
import org.springframework.data.relational.core.sql.Table;
import org.springframework.util.Assert;
/**
* Oracle MERGE upsert. Uses {@code SELECT ... FROM DUAL} as the source, which Oracle requires for
* literal/bind-marker-only selects.
* Oracle MERGE upsert. Uses {@code SELECT ... FROM DUAL} for source values.
*
* @since 4.x
*/
@ -34,53 +31,56 @@ public enum OracleUpsertRenderContext implements UpsertRenderContext { @@ -34,53 +31,56 @@ public enum OracleUpsertRenderContext implements UpsertRenderContext {
INSTANCE;
private static final String SOURCE_FROM_CLAUSE = " FROM DUAL";
@Override
public String renderUpsert(Table table, Columns merge, Function<SqlIdentifier, String> bindMarkerFn) {
public String renderUpsert(Table table, Columns columns, Function<SqlIdentifier, String> bindMarkerFn) {
List<SqlIdentifier> insertColumns = merge.insertColumns();
List<SqlIdentifier> conflictColumns = merge.filterColumns();
IdentifierProcessing identifierProcessing = merge.identifierProcessing();
Assert.notEmpty(columns.insertColumns(), "Insert columns must not be empty");
Assert.notEmpty(columns.filterColumns(), "Filter columns must not be empty");
Assert.notEmpty(insertColumns, "Insert columns must not be empty");
Assert.notEmpty(conflictColumns, "Conflict columns must not be empty");
String targetTableAlias = columns.identifierProcessing().quote(StandardSqlUpsertRenderContext.targetTableAlias);
String sourceTableAlias = columns.identifierProcessing().quote(StandardSqlUpsertRenderContext.sourceTableAlias);
Set<SqlIdentifier> conflictSet = Set.copyOf(conflictColumns);
String tableSql = table.getName().toSql(identifierProcessing);
String tableName = columns.tableName(table);
String insertColumnNames = String.join(", ", columns.insertColumnNames());
String sourceSelectList = String.join(", ",
columns.insertColumns().stream().map(col -> bindMarkerFn.apply(col) + " AS " + columns.column(col)).toList());
String sourceSelectList = String.join(", ", insertColumns.stream()
.map(col -> bindMarkerFn.apply(col) + " AS " + col.toSql(identifierProcessing))
.toList());
String onCondition = String.join(" AND ", columns.filterColumns().stream().map(col -> {
String colName = columns.column(col);
return "%s.%s = %s.%s".formatted(targetTableAlias, colName, sourceTableAlias, colName);
}).toList());
String onCondition = "(" + String.join(" AND ", conflictColumns.stream()
.map(col -> "t." + col.toSql(identifierProcessing) + " = s." + col.toSql(identifierProcessing))
.toList()) + ")";
String insertValuesSql = String.join(", ",
columns.insertColumns().stream().map(col -> columns.column(sourceTableAlias, col)).toList());
List<SqlIdentifier> updateColumns = insertColumns.stream()
.filter(col -> !conflictSet.contains(col))
.toList();
String insertClause = "WHEN NOT MATCHED THEN INSERT (%s) VALUES (%s)".formatted(insertColumnNames,
insertValuesSql);
String updateSetClause;
List<SqlIdentifier> updateColumns = columns.updateColumns();
if (updateColumns.isEmpty()) {
SqlIdentifier firstConflict = conflictColumns.get(0);
updateSetClause = "t." + firstConflict.toSql(identifierProcessing) + " = s."
+ firstConflict.toSql(identifierProcessing);
} else {
updateSetClause = String.join(", ", updateColumns.stream()
.map(col -> "t." + col.toSql(identifierProcessing) + " = s." + col.toSql(identifierProcessing))
.toList());
// ORA-38104: columns referenced in ON cannot be updated; omit WHEN MATCHED so existing rows are left
// unchanged (same as a no-op update of key-only columns).
return "MERGE INTO %s %s USING (SELECT %s FROM DUAL) %s ON (%s) %s".formatted( //
tableName, //
targetTableAlias, //
sourceSelectList, //
sourceTableAlias, //
onCondition, //
insertClause);
}
String insertColumnsSql = String.join(", ",
insertColumns.stream().map(col -> col.toSql(identifierProcessing)).toList());
String insertValuesSql = String.join(", ",
insertColumns.stream().map(col -> "s." + col.toSql(identifierProcessing)).toList());
return "MERGE INTO " + tableSql + " t USING (SELECT " + sourceSelectList + SOURCE_FROM_CLAUSE + ") s ON "
+ onCondition
+ " WHEN MATCHED THEN UPDATE SET " + updateSetClause
+ " WHEN NOT MATCHED THEN INSERT (" + insertColumnsSql + ") VALUES (" + insertValuesSql + ")";
String updateSetClause = String.join(", ", updateColumns.stream().map(col -> {
String colName = columns.column(col);
return "%s.%s = %s.%s".formatted(targetTableAlias, colName, sourceTableAlias, colName);
}).toList());
return "MERGE INTO %s %s USING (SELECT %s FROM DUAL) %s ON (%s) WHEN MATCHED THEN UPDATE SET %s %s".formatted( //
tableName, //
targetTableAlias, //
sourceSelectList, //
sourceTableAlias, //
onCondition, //
updateSetClause, //
insertClause);
}
}

10
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/PostgresUpsertRenderContext.java

@ -42,8 +42,16 @@ public enum PostgresUpsertRenderContext implements UpsertRenderContext { @@ -42,8 +42,16 @@ public enum PostgresUpsertRenderContext implements UpsertRenderContext {
String insertColumnNames = String.join(", ", columns.insertColumnNames());
String bindMarkers = String.join(", ", columns.insertColumnBindMarkers(bindMarkerFn));
String filterColumnNames = String.join(", ", columns.filterColumnNames());
String setValues = setValuesSnippet(columns);
if(columns.updateColumns().isEmpty()) {
return "INSERT INTO %s (%s) VALUES (%s) ON CONFLICT (%s) DO NOTHING".formatted(//
tableName, //
insertColumnNames, //
bindMarkers, //
filterColumnNames);
}
String setValues = setValuesSnippet(columns);
return "INSERT INTO %s (%s) VALUES (%s) ON CONFLICT (%s) DO UPDATE SET %s".formatted(//
tableName, //
insertColumnNames, //

45
spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/render/StandardSqlUpsertRenderContext.java

@ -33,8 +33,8 @@ public enum StandardSqlUpsertRenderContext implements UpsertRenderContext { @@ -33,8 +33,8 @@ public enum StandardSqlUpsertRenderContext implements UpsertRenderContext {
INSTANCE;
private static final String targetTableAlias = "_t";
private static final String sourceTableAlias = "_s";
static final String targetTableAlias = "_t";
static final String sourceTableAlias = "_s";
@Override
public String renderUpsert(Table table, Columns columns, Function<SqlIdentifier, String> bindMarkerFn) {
@ -54,26 +54,39 @@ public enum StandardSqlUpsertRenderContext implements UpsertRenderContext { @@ -54,26 +54,39 @@ public enum StandardSqlUpsertRenderContext implements UpsertRenderContext {
return "%s.%s = %s.%s".formatted(targetTableAlias, colName, sourceTableAlias, colName);
}).toList());
String insertValuesSql = String.join(", ",
columns.insertColumns().stream().map(col -> columns.column(sourceTableAlias, col)).toList());
String insertClause = "WHEN NOT MATCHED THEN INSERT (%s) VALUES (%s)".formatted(insertColumnNames,
insertValuesSql);
List<SqlIdentifier> updateColumns = columns.updateColumns();
if (updateColumns.isEmpty()) {
// Matched rows are left unchanged. Updating only key columns is invalid on SQL Server (identity) and Oracle
// (ORA-38104).
return "MERGE INTO %s %s USING (VALUES (%s)) AS %s (%s) ON %s %s".formatted( //
tableName, //
targetTableAlias, //
bindMarkers, //
sourceTableAlias, //
insertColumnNames, //
onCondition, //
insertClause);
}
String updateSetClause = String.join(", ", updateColumns.stream().map(col -> {
String colName = columns.column(col);
return "%s.%s = %s.%s".formatted(targetTableAlias, colName, sourceTableAlias, colName);
}).toList());
String insertValuesSql = String.join(", ",
columns.insertColumns().stream().map(col -> columns.column(sourceTableAlias, col)).toList());
return "MERGE INTO %s %s USING (VALUES (%s)) AS %s (%s) ON %s WHEN MATCHED THEN UPDATE SET %s WHEN NOT MATCHED THEN INSERT (%s) VALUES (%s)"
.formatted( //
tableName, //
targetTableAlias, //
bindMarkers, //
sourceTableAlias, //
insertColumnNames, //
onCondition, //
updateSetClause, //
insertColumnNames, //
insertValuesSql);
return "MERGE INTO %s %s USING (VALUES (%s)) AS %s (%s) ON %s WHEN MATCHED THEN UPDATE SET %s %s".formatted( //
tableName, //
targetTableAlias, //
bindMarkers, //
sourceTableAlias, //
insertColumnNames, //
onCondition, //
updateSetClause, //
insertClause);
}
}

59
spring-data-relational/src/test/java/org/springframework/data/relational/core/sql/render/UpsertRenderContextUnitTests.java

@ -48,6 +48,23 @@ class UpsertRenderContextUnitTests { @@ -48,6 +48,23 @@ class UpsertRenderContextUnitTests {
"MERGE INTO my_table \"_t\" USING (VALUES (:id, :name)) AS \"_s\" (id, name) ON \"_t\".id = \"_s\".id WHEN MATCHED THEN UPDATE SET \"_t\".name = \"_s\".name WHEN NOT MATCHED THEN INSERT (id, name) VALUES (\"_s\".id, \"_s\".name)");
}
@Test // GH-493
void mergeUpsertWithMultipleConflictColumnsBuildsFilterClauseWithAllColumns() {
List<SqlIdentifier> insertColumns = List.of(SqlIdentifier.unquoted("tenant_id"), SqlIdentifier.unquoted("id"),
SqlIdentifier.unquoted("name"));
List<SqlIdentifier> conflictColumns = List.of(SqlIdentifier.unquoted("tenant_id"), SqlIdentifier.unquoted("id"));
Columns columns = new Columns(insertColumns, conflictColumns, IDENTIFIER_PROCESSING);
String sql = StandardSqlUpsertRenderContext.INSTANCE.renderUpsert(TABLE, columns, BIND_MARKER);
assertThat(sql).isEqualToIgnoringWhitespace(
"MERGE INTO my_table \"_t\" USING (VALUES (:tenant_id, :id, :name)) AS \"_s\" (tenant_id, id, name) "
+ "ON \"_t\".tenant_id = \"_s\".tenant_id AND \"_t\".id = \"_s\".id "
+ "WHEN MATCHED THEN UPDATE SET \"_t\".name = \"_s\".name "
+ "WHEN NOT MATCHED THEN INSERT (tenant_id, id, name) VALUES (\"_s\".tenant_id, \"_s\".id, \"_s\".name)");
}
@Test // GH-493
void postgresUpsertRendersInsertOnConflictDoUpdate() {
@ -58,6 +75,16 @@ class UpsertRenderContextUnitTests { @@ -58,6 +75,16 @@ class UpsertRenderContextUnitTests {
"INSERT INTO my_table (id, name) VALUES (:id, :name) ON CONFLICT (id) DO UPDATE SET name = EXCLUDED.name");
}
@Test // GH-493
void postgresUpsertRendersInsertOnConflictDoNothing() {
String sql = PostgresUpsertRenderContext.INSTANCE.renderUpsert(TABLE,
new Columns(INSERT_COLUMNS, INSERT_COLUMNS, IDENTIFIER_PROCESSING), BIND_MARKER);
assertThat(sql).isEqualToIgnoringWhitespace(
"INSERT INTO my_table (id, name) VALUES (:id, :name) ON CONFLICT (id, name) DO NOTHING");
}
@Test // GH-493
void mySqlUpsertRendersOnDuplicateKeyUpdate() {
@ -85,14 +112,30 @@ class UpsertRenderContextUnitTests { @@ -85,14 +112,30 @@ class UpsertRenderContextUnitTests {
String sql = OracleUpsertRenderContext.INSTANCE.renderUpsert(TABLE,
new Columns(INSERT_COLUMNS, CONFLICT_COLUMNS, IDENTIFIER_PROCESSING), BIND_MARKER);
System.out.println("sql: " + sql);
assertThat(sql).startsWith("MERGE INTO");
assertThat(sql).contains("USING (SELECT");
assertThat(sql).contains("FROM DUAL");
// Oracle requires ON condition in parentheses (ORA-00969 otherwise).
assertThat(sql).contains("ON (t.id = s.id)");
assertThat(sql).contains("WHEN MATCHED THEN UPDATE SET");
assertThat(sql).contains("WHEN NOT MATCHED THEN INSERT");
assertThat(sql).isEqualToIgnoringWhitespace(
"MERGE INTO my_table \"_t\" USING (SELECT :id AS id, :name AS name FROM DUAL) \"_s\" ON (\"_t\".id = \"_s\".id) WHEN MATCHED THEN UPDATE SET \"_t\".name = \"_s\".name WHEN NOT MATCHED THEN INSERT (id, name) VALUES (\"_s\".id, \"_s\".name)");
}
@Test // GH-493
void standardMergeIdOnlyOmitsWhenMatchedUpdate() {
List<SqlIdentifier> idOnly = List.of(SqlIdentifier.unquoted("id"));
String sql = StandardSqlUpsertRenderContext.INSTANCE.renderUpsert(TABLE, new Columns(idOnly, idOnly,
IDENTIFIER_PROCESSING), BIND_MARKER);
assertThat(sql).isEqualTo(
"MERGE INTO my_table \"_t\" USING (VALUES (:id)) AS \"_s\" (id) ON \"_t\".id = \"_s\".id WHEN NOT MATCHED THEN INSERT (id) VALUES (\"_s\".id)");
}
@Test // GH-493
void oracleMergeIdOnlyOmitsWhenMatchedUpdate() {
List<SqlIdentifier> idOnly = List.of(SqlIdentifier.unquoted("id"));
String sql = OracleUpsertRenderContext.INSTANCE.renderUpsert(TABLE, new Columns(idOnly, idOnly,
IDENTIFIER_PROCESSING), BIND_MARKER);
assertThat(sql).isEqualToIgnoringWhitespace(
"MERGE INTO my_table \"_t\" USING (SELECT :id AS id FROM DUAL) \"_s\" ON (\"_t\".id = \"_s\".id) WHEN NOT MATCHED THEN INSERT (id) VALUES (\"_s\".id)");
}
@Test // GH-493

Loading…
Cancel
Save