From 71330ddb0f1a1c5b10b462e9825760cbc69015f0 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 24 Oct 2023 11:50:12 +0200 Subject: [PATCH] Revise support for quoted identifiers in SimpleJdbcInsert Closes gh-31208 --- .../core/metadata/TableMetaDataContext.java | 6 +-- .../jdbc/core/simple/AbstractJdbcInsert.java | 4 ++ .../simple/SimpleJdbcInsertOperations.java | 14 ++++++ .../SimpleJdbcInsertIntegrationTests.java | 45 +++++++++---------- .../core/simple/SimpleJdbcInsertTests.java | 14 ++++++ 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java index e9960aa89c9..601a6c33405 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java @@ -312,7 +312,7 @@ public class TableMetaDataContext { if (schemaName != null) { if (quoting) { insertStatement.append(identifierQuoteString); - insertStatement.append(this.metaDataProvider.schemaNameToUse(schemaName)); + insertStatement.append(schemaName); insertStatement.append(identifierQuoteString); } else { @@ -324,7 +324,7 @@ public class TableMetaDataContext { String tableName = getTableName(); if (quoting) { insertStatement.append(identifierQuoteString); - insertStatement.append(this.metaDataProvider.tableNameToUse(tableName)); + insertStatement.append(tableName); insertStatement.append(identifierQuoteString); } else { @@ -341,7 +341,7 @@ public class TableMetaDataContext { } if (quoting) { insertStatement.append(identifierQuoteString); - insertStatement.append(this.metaDataProvider.columnNameToUse(columnName)); + insertStatement.append(columnName); insertStatement.append(identifierQuoteString); } else { diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcInsert.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcInsert.java index ebe98218afb..979ae771a56 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcInsert.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcInsert.java @@ -275,6 +275,10 @@ public abstract class AbstractJdbcInsert { if (getTableName() == null) { throw new InvalidDataAccessApiUsageException("Table name is required"); } + if (isQuoteIdentifiers() && this.declaredColumns.isEmpty()) { + throw new InvalidDataAccessApiUsageException( + "Explicit column names must be provided when using quoted identifiers"); + } try { this.jdbcTemplate.afterPropertiesSet(); } diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertOperations.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertOperations.java index b2e611b704f..2338ef788ea 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertOperations.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertOperations.java @@ -73,9 +73,23 @@ public interface SimpleJdbcInsertOperations { *

If this method is invoked, the identifier quote string for the underlying * database will be used to quote SQL identifiers in generated SQL statements. * In this context, SQL identifiers refer to schema, table, and column names. + *

When identifiers are quoted, explicit column names must be supplied via + * {@link #usingColumns(String...)}. Furthermore, all identifiers for the + * schema name, table name, and column names must match the corresponding + * identifiers in the database's metadata regarding casing (mixed case, + * uppercase, or lowercase). * @return this {@code SimpleJdbcInsert} (for method chaining) * @since 6.1 + * @see #withSchemaName(String) + * @see #withTableName(String) + * @see #usingColumns(String...) * @see java.sql.DatabaseMetaData#getIdentifierQuoteString() + * @see java.sql.DatabaseMetaData#storesMixedCaseIdentifiers() + * @see java.sql.DatabaseMetaData#storesMixedCaseQuotedIdentifiers() + * @see java.sql.DatabaseMetaData#storesUpperCaseIdentifiers() + * @see java.sql.DatabaseMetaData#storesUpperCaseQuotedIdentifiers() + * @see java.sql.DatabaseMetaData#storesLowerCaseIdentifiers() + * @see java.sql.DatabaseMetaData#storesLowerCaseQuotedIdentifiers() */ SimpleJdbcInsertOperations usingQuotedIdentifiers(); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertIntegrationTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertIntegrationTests.java index dbcb4d4b068..52b8977a2db 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertIntegrationTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertIntegrationTests.java @@ -56,25 +56,13 @@ class SimpleJdbcInsertIntegrationTests { insertJaneSmith(insert); } - @Test // gh-24013 - void retrieveColumnNamesFromMetadataAndUsingQuotedIdentifiers() throws Exception { - SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase) - .withTableName("users") - .usingGeneratedKeyColumns("id") - .usingQuotedIdentifiers(); - - insert.compile(); - // NOTE: quoted identifiers in H2/HSQL will be UPPERCASE! - assertThat(insert.getInsertString()).isEqualTo("INSERT INTO \"USERS\" (\"FIRST_NAME\", \"LAST_NAME\") VALUES(?, ?)"); - - insertJaneSmith(insert); - } - @Test void usingColumns() { SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase) + .withoutTableColumnMetaDataAccess() .withTableName("users") - .usingColumns("first_name", "last_name"); + .usingColumns("first_name", "last_name") + .usingGeneratedKeyColumns("id"); insert.compile(); assertThat(insert.getInsertString()).isEqualTo("INSERT INTO users (first_name, last_name) VALUES(?, ?)"); @@ -84,13 +72,16 @@ class SimpleJdbcInsertIntegrationTests { @Test // gh-24013 void usingColumnsAndQuotedIdentifiers() throws Exception { + // NOTE: unquoted identifiers in H2/HSQL must be converted to UPPERCASE + // since that's how they are stored in the DB metadata. SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase) - .withTableName("users") - .usingColumns("first_name", "last_name") + .withoutTableColumnMetaDataAccess() + .withTableName("USERS") + .usingColumns("FIRST_NAME", "LAST_NAME") + .usingGeneratedKeyColumns("id") .usingQuotedIdentifiers(); insert.compile(); - // NOTE: quoted identifiers in H2/HSQL will be UPPERCASE! assertThat(insert.getInsertString()).isEqualToIgnoringNewLines(""" INSERT INTO "USERS" ("FIRST_NAME", "LAST_NAME") VALUES(?, ?) """); @@ -116,9 +107,11 @@ class SimpleJdbcInsertIntegrationTests { @Test void usingColumnsWithSchemaName() { SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase) + .withoutTableColumnMetaDataAccess() .withSchemaName("my_schema") .withTableName("users") - .usingColumns("first_name", "last_name"); + .usingColumns("first_name", "last_name") + .usingGeneratedKeyColumns("id"); insert.compile(); assertThat(insert.getInsertString()).isEqualTo("INSERT INTO my_schema.users (first_name, last_name) VALUES(?, ?)"); @@ -128,14 +121,17 @@ class SimpleJdbcInsertIntegrationTests { @Test // gh-24013 void usingColumnsAndQuotedIdentifiersWithSchemaName() throws Exception { + // NOTE: unquoted identifiers in H2/HSQL must be converted to UPPERCASE + // since that's how they are stored in the DB metadata. SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase) - .withSchemaName("my_schema") - .withTableName("users") - .usingColumns("first_name", "last_name") + .withoutTableColumnMetaDataAccess() + .withSchemaName("MY_SCHEMA") + .withTableName("USERS") + .usingColumns("FIRST_NAME", "LAST_NAME") + .usingGeneratedKeyColumns("id") .usingQuotedIdentifiers(); insert.compile(); - // NOTE: quoted identifiers in H2/HSQL will be UPPERCASE! assertThat(insert.getInsertString()).isEqualToIgnoringNewLines(""" INSERT INTO "MY_SCHEMA"."USERS" ("FIRST_NAME", "LAST_NAME") VALUES(?, ?) """); @@ -182,7 +178,8 @@ class SimpleJdbcInsertIntegrationTests { } protected void insertJaneSmith(SimpleJdbcInsert insert) { - insert.execute(Map.of("first_name", "Jane", "last_name", "Smith")); + Number id = insert.executeAndReturnKey(Map.of("first_name", "Jane", "last_name", "Smith")); + assertThat(id.intValue()).isEqualTo(2); assertNumUsers(2); } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java index 6e61f03f2de..dcb4305d928 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java @@ -77,6 +77,20 @@ class SimpleJdbcInsertTests { connection.close(); } + @Test // gh-24013 and gh-31208 + void usingQuotedIdentifiersWithoutSupplyingColumnNames() throws Exception { + SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource) + .withTableName("my_table") + .usingQuotedIdentifiers(); + + assertThatExceptionOfType(InvalidDataAccessApiUsageException.class) + .isThrownBy(insert::compile) + .withMessage("Explicit column names must be provided when using quoted identifiers"); + + // Appease the @AfterEach checks. + connection.close(); + } + /** * This method does not test any functionality but rather only that * configuration methods can be chained without compiler errors.