Revise quoted identifier support in SimpleJdbcInsert

Prior to this commit and the previous commit, SimpleJdbcInsert did not
provide built-in support for "quoted identifiers". Consequently, if any
column names conflicted with keywords or functions from the underlying
database, you had to manually quote the column names when specifying
them via `usingColumns(...)`, and there was unfortunately no way to
quote schema and table names.

The previous commit provided rudimentary support for quoted SQL
identifiers (schema, table, and column names) by querying
java.sql.DatabaseMetaData.getIdentifierQuoteString() to determine the
quote string. It also introduced `usingEscaping(boolean)` in
`SimpleJdbcInsertOperations` to enable the feature. However, it
incorrectly quoted the schema and table names together, and it did not
take into account the fact that a quoted identifier should respect the
casing (uppercase vs. lowercase) of the underlying database's metadata.

This commit revises quoted identifier support in `SimpleJdbcInsert` by:

- renaming `usingEscaping(boolean)` to `usingQuotedIdentifiers()`

- quoting schema and table names separately

- respecting the casing (uppercase vs. lowercase) of the underlying
  database's metadata when quoting identifiers

- introducing integration tests against an in-memory H2 database

See gh-13874
Closes gh-24013

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/GenericTableMetaDataProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -77,8 +77,8 @@ public class GenericTableMetaDataProvider implements TableMetaDataProvider {
 	/** Collection of TableParameterMetaData objects. */
 	private final List<TableParameterMetaData> tableParameterMetaData = new ArrayList<>();
 
-	/** the string used to quote SQL identifiers. */
+	/** The string used to quote SQL identifiers. */
-	private String identifierQuoteString = "";
+	private String identifierQuoteString = " ";
 
 	/**
 	 * Constructor used to initialize with provided database meta-data.
@@ -305,7 +305,9 @@ public class GenericTableMetaDataProvider implements TableMetaDataProvider {
 	}
 
 	/**
-	 * Provide access to identifier quote string.
+	 * Provide access to the identifier quote string.
+	 * @since 6.1
+	 * @see java.sql.DatabaseMetaData#getIdentifierQuoteString()
 	 */
 	@Override
 	public String getIdentifierQuoteString() {

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@ import org.springframework.jdbc.support.JdbcUtils;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * Class to manage context meta-data used for the configuration
@@ -72,16 +73,16 @@ public class TableMetaDataContext {
 	// Should we override default for including synonyms for meta-data lookups
 	private boolean overrideIncludeSynonymsDefault = false;
 
+	// Are we using generated key columns?
+	private boolean generatedKeyColumnsUsed = false;
+
+	// Are we quoting identifiers?
+	private boolean quoteIdentifiers = false;
+
 	// The provider of table meta-data
 	@Nullable
 	private TableMetaDataProvider metaDataProvider;
 
-	// Are we using generated key columns
-	private boolean generatedKeyColumnsUsed = false;
-
-	// Are we using escaping for SQL identifiers
-	private boolean usingEscaping = false;
-
 
 	/**
 	 * Set the name of the table for this context.
@@ -142,7 +143,6 @@ public class TableMetaDataContext {
 		return this.accessTableColumnMetaData;
 	}
 
-
 	/**
 	 * Specify whether we should override default for accessing synonyms.
 	 */
@@ -157,6 +157,28 @@ public class TableMetaDataContext {
 		return this.overrideIncludeSynonymsDefault;
 	}
 
+	/**
+	 * Specify whether we are quoting SQL identifiers.
+	 * <p>Defaults to {@code false}. If set to {@code true}, the identifier
+	 * quote string for the underlying database will be used to quote SQL
+	 * identifiers in generated SQL statements.
+	 * @param quoteIdentifiers whether identifiers should be quoted
+	 * @since 6.1
+	 * @see java.sql.DatabaseMetaData#getIdentifierQuoteString()
+	 */
+	public void setQuoteIdentifiers(boolean quoteIdentifiers) {
+		this.quoteIdentifiers = quoteIdentifiers;
+	}
+
+	/**
+	 * Are we quoting identifiers?
+	 * @since 6.1
+	 * @see #setQuoteIdentifiers(boolean)
+	 */
+	public boolean isQuoteIdentifiers() {
+		return this.quoteIdentifiers;
+	}
+
 	/**
 	 * Get a List of the table column names.
 	 */
@@ -269,7 +291,6 @@ public class TableMetaDataContext {
 		return values;
 	}
 
-
 	/**
 	 * Build the insert string based on configuration and meta-data information.
 	 * @return the insert string to be used
@@ -279,24 +300,37 @@ public class TableMetaDataContext {
 		for (String key : generatedKeyNames) {
 			keys.add(key.toUpperCase());
 		}
-		String identifierQuoteString = "";
+
-		if (this.metaDataProvider != null && this.usingEscaping) {
+		String identifierQuoteString = (isQuoteIdentifiers() ?
-			identifierQuoteString = this.metaDataProvider.getIdentifierQuoteString();
+				obtainMetaDataProvider().getIdentifierQuoteString() : null);
-		}
+		boolean quoting = StringUtils.hasText(identifierQuoteString);
+
 		StringBuilder insertStatement = new StringBuilder();
 		insertStatement.append("INSERT INTO ");
-		if (getSchemaName() != null) {
+
-			insertStatement.append(identifierQuoteString);
+		String schemaName = getSchemaName();
-			insertStatement.append(getSchemaName());
+		if (schemaName != null) {
+			if (quoting) {
+				insertStatement.append(identifierQuoteString);
+				insertStatement.append(this.metaDataProvider.schemaNameToUse(schemaName));
+				insertStatement.append(identifierQuoteString);
+			}
+			else {
+				insertStatement.append(schemaName);
+			}
 			insertStatement.append('.');
-			insertStatement.append(getTableName());
-			insertStatement.append(identifierQuoteString);
 		}
-		else {
+
+		String tableName = getTableName();
+		if (quoting) {
 			insertStatement.append(identifierQuoteString);
-			insertStatement.append(getTableName());
+			insertStatement.append(this.metaDataProvider.tableNameToUse(tableName));
 			insertStatement.append(identifierQuoteString);
 		}
+		else {
+			insertStatement.append(tableName);
+		}
+
 		insertStatement.append(" (");
 		int columnCount = 0;
 		for (String columnName : getTableColumns()) {
@@ -305,9 +339,14 @@ public class TableMetaDataContext {
 				if (columnCount > 1) {
 					insertStatement.append(", ");
 				}
-				insertStatement.append(identifierQuoteString);
+				if (quoting) {
-				insertStatement.append(columnName);
+					insertStatement.append(identifierQuoteString);
-				insertStatement.append(identifierQuoteString);
+					insertStatement.append(this.metaDataProvider.columnNameToUse(columnName));
+					insertStatement.append(identifierQuoteString);
+				}
+				else {
+					insertStatement.append(columnName);
+				}
 			}
 		}
 		insertStatement.append(") VALUES(");
@@ -315,11 +354,11 @@ public class TableMetaDataContext {
 			if (this.generatedKeyColumnsUsed) {
 				if (logger.isDebugEnabled()) {
 					logger.debug("Unable to locate non-key columns for table '" +
-							getTableName() + "' so an empty insert statement is generated");
+							tableName + "' so an empty insert statement is generated");
 				}
 			}
 			else {
-				String message = "Unable to locate columns for table '" + getTableName()
+				String message = "Unable to locate columns for table '" + tableName
 						+ "' so an insert statement can't be generated.";
 				if (isAccessTableColumnMetaData()) {
 					message += " Consider specifying explicit column names -- for example, via SimpleJdbcInsert#usingColumns().";
@@ -397,11 +436,4 @@ public class TableMetaDataContext {
 		return obtainMetaDataProvider().isGeneratedKeysColumnNameArraySupported();
 	}
 
-	public boolean isUsingEscaping() {
-		return this.usingEscaping;
-	}
-
-	public void setUsingEscaping(boolean usingEscaping) {
-		this.usingEscaping = usingEscaping;
-	}
 }

spring-jdbc/src/main/java/org/springframework/jdbc/core/metadata/TableMetaDataProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -140,9 +140,12 @@ public interface TableMetaDataProvider {
 	List<TableParameterMetaData> getTableParameterMetaData();
 
 	/**
-	 * Retrieves the string used to quote SQL identifiers. This method returns a space " " if identifier quoting is not supported.
+	 * Get the string used to quote SQL identifiers.
-	 * {@link DatabaseMetaData#getIdentifierQuoteString()}
+	 * <p>This method returns a space ({@code " "}) if identifier quoting is not
-	 * @return database identifier quote string.
+	 * supported.
+	 * @return database identifier quote string
+	 * @since 6.1
+	 * @see DatabaseMetaData#getIdentifierQuoteString()
 	 */
 	String getIdentifierQuoteString();
 

spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/AbstractJdbcInsert.java

@@ -237,17 +237,25 @@ public abstract class AbstractJdbcInsert {
 	}
 
 	/**
-	 * Set using using escaping.
+	 * Specify whether SQL identifiers should be quoted.
+	 * <p>Defaults to {@code false}. If set to {@code true}, the identifier
+	 * quote string for the underlying database will be used to quote SQL
+	 * identifiers in generated SQL statements.
+	 * @param quoteIdentifiers whether identifiers should be quoted
+	 * @since 6.1
+	 * @see java.sql.DatabaseMetaData#getIdentifierQuoteString()
 	 */
-	public void setUsingEscaping(boolean usingEscaping) {
+	public void setQuoteIdentifiers(boolean quoteIdentifiers) {
-		this.tableMetaDataContext.setUsingEscaping(usingEscaping);
+		this.tableMetaDataContext.setQuoteIdentifiers(quoteIdentifiers);
 	}
 
 	/**
-	 * Get using escaping.
+	 * Get the {@code quoteIdentifiers} flag.
+	 * @since 6.1
+	 * @see #setQuoteIdentifiers(boolean)
 	 */
-	public boolean isUsingEscaping() {
+	public boolean isQuoteIdentifiers() {
-		return this.tableMetaDataContext.isUsingEscaping();
+		return this.tableMetaDataContext.isQuoteIdentifiers();
 	}
 
 

spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsert.java

@@ -103,20 +103,20 @@ public class SimpleJdbcInsert extends AbstractJdbcInsert implements SimpleJdbcIn
 	}
 
 	@Override
-	public SimpleJdbcInsertOperations withoutTableColumnMetaDataAccess() {
+	public SimpleJdbcInsert usingQuotedIdentifiers() {
-		setAccessTableColumnMetaData(false);
+		setQuoteIdentifiers(true);
 		return this;
 	}
 
 	@Override
-	public SimpleJdbcInsertOperations includeSynonymsForTableColumnMetaData() {
+	public SimpleJdbcInsertOperations withoutTableColumnMetaDataAccess() {
-		setOverrideIncludeSynonymsDefault(true);
+		setAccessTableColumnMetaData(false);
 		return this;
 	}
 
 	@Override
-	public SimpleJdbcInsert usingEscaping(boolean usingEscaping) {
+	public SimpleJdbcInsertOperations includeSynonymsForTableColumnMetaData() {
-		setUsingEscaping(usingEscaping);
+		setOverrideIncludeSynonymsDefault(true);
 		return this;
 	}
 

spring-jdbc/src/main/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertOperations.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -68,6 +68,17 @@ public interface SimpleJdbcInsertOperations {
 	 */
 	SimpleJdbcInsertOperations usingGeneratedKeyColumns(String... columnNames);
 
+	/**
+	 * Specify that SQL identifiers should be quoted.
+	 * <p>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.
+	 * @return this {@code SimpleJdbcInsert} (for method chaining)
+	 * @since 6.1
+	 * @see java.sql.DatabaseMetaData#getIdentifierQuoteString()
+	 */
+	SimpleJdbcInsertOperations usingQuotedIdentifiers();
+
 	/**
 	 * Turn off any processing of column meta-data information obtained via JDBC.
 	 * @return this {@code SimpleJdbcInsert} (for method chaining)
@@ -82,14 +93,6 @@ public interface SimpleJdbcInsertOperations {
 	 */
 	SimpleJdbcInsertOperations includeSynonymsForTableColumnMetaData();
 
-	/**
-	 * Specify should sql identifiers be quoted.
-	 * @param usingEscaping should sql identifiers be quoted
-	 * @return the instance of this SimpleJdbcInsert
-	 */
-	SimpleJdbcInsertOperations usingEscaping(boolean usingEscaping);
-
-
 	/**
 	 * Execute the insert using the values passed in.
 	 * @param args a Map containing column names and corresponding value

spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertIntegrationTests.java

@@ -0,0 +1,181 @@
+/*
+ * Copyright 2002-2023 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.jdbc.core.simple;
+
+import java.util.Map;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.core.io.ClassRelativeResourceLoader;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+import org.springframework.jdbc.datasource.init.DatabasePopulator;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests for {@link SimpleJdbcInsert} using an embedded H2 database.
+ *
+ * @author Sam Brannen
+ * @since 6.1
+ */
+class SimpleJdbcInsertIntegrationTests {
+
+	@Nested
+	class DefaultSchemaTests extends AbstractSimpleJdbcInsertIntegrationTests {
+
+		@Test
+		void retrieveColumnNamesFromMetadata() throws Exception {
+			SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase)
+					.withTableName("users")
+					.usingGeneratedKeyColumns("id");
+
+			insert.compile();
+			// NOTE: column names looked up via metadata 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)
+					.withTableName("users")
+					.usingColumns("first_name", "last_name");
+
+			insert.compile();
+			assertThat(insert.getInsertString()).isEqualTo("INSERT INTO users (first_name, last_name) VALUES(?, ?)");
+
+			insertJaneSmith(insert);
+		}
+
+		@Test  //  gh-24013
+		void usingColumnsAndQuotedIdentifiers() throws Exception {
+			SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase)
+					.withTableName("users")
+					.usingColumns("first_name", "last_name")
+					.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);
+		}
+
+		@Override
+		protected String getSchemaScript() {
+			return "users-schema.sql";
+		}
+
+		@Override
+		protected String getUsersTableName() {
+			return "users";
+		}
+
+	}
+
+	@Nested
+	class CustomSchemaTests extends AbstractSimpleJdbcInsertIntegrationTests {
+
+		@Test
+		void usingColumnsWithSchemaName() {
+			SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase)
+					.withSchemaName("my_schema")
+					.withTableName("users")
+					.usingColumns("first_name", "last_name");
+
+			insert.compile();
+			assertThat(insert.getInsertString()).isEqualTo("INSERT INTO my_schema.users (first_name, last_name) VALUES(?, ?)");
+
+			insertJaneSmith(insert);
+		}
+
+		@Test  //  gh-24013
+		void usingColumnsAndQuotedIdentifiersWithSchemaName() throws Exception {
+			SimpleJdbcInsert insert = new SimpleJdbcInsert(embeddedDatabase)
+					.withSchemaName("my_schema")
+					.withTableName("users")
+					.usingColumns("first_name", "last_name")
+					.usingQuotedIdentifiers();
+
+			insert.compile();
+			// NOTE: quoted identifiers in H2/HSQL will be UPPERCASE!
+			assertThat(insert.getInsertString()).isEqualTo("INSERT INTO \"MY_SCHEMA\".\"USERS\" (\"FIRST_NAME\", \"LAST_NAME\") VALUES(?, ?)");
+
+			insertJaneSmith(insert);
+		}
+
+		@Override
+		protected String getSchemaScript() {
+			return "users-schema-with-custom-schema.sql";
+		}
+
+		@Override
+		protected String getUsersTableName() {
+			return "my_schema.users";
+		}
+
+	}
+
+	private static abstract class AbstractSimpleJdbcInsertIntegrationTests {
+
+		protected EmbeddedDatabase embeddedDatabase;
+
+
+		protected abstract String getSchemaScript();
+
+		protected abstract String getUsersTableName();
+
+		protected EmbeddedDatabase createEmbeddedDatabase() {
+			return new EmbeddedDatabaseBuilder(new ClassRelativeResourceLoader(DatabasePopulator.class))
+					.setType(EmbeddedDatabaseType.H2)
+					.addScript(getSchemaScript())
+					.addScript("users-data.sql")
+					.build();
+		}
+
+
+		@BeforeEach
+		void checkDatabaseSetup() {
+			this.embeddedDatabase = createEmbeddedDatabase();
+			assertNumUsers(1);
+		}
+
+		@AfterEach
+		void shutdown() {
+			this.embeddedDatabase.shutdown();
+		}
+
+		protected void assertNumUsers(long count) {
+			JdbcClient jdbcClient = JdbcClient.create(this.embeddedDatabase);
+			long numUsers = jdbcClient.sql("select count(*) from " + getUsersTableName()).query().singleValue();
+			assertThat(numUsers).isEqualTo(count);
+		}
+
+		protected void insertJaneSmith(SimpleJdbcInsert insert) {
+			insert.execute(Map.of("first_name", "Jane", "last_name", "Smith"));
+			assertNumUsers(2);
+		}
+
+	}
+
+}

spring-jdbc/src/test/java/org/springframework/jdbc/core/simple/SimpleJdbcInsertTests.java

@@ -139,36 +139,41 @@ class SimpleJdbcInsertTests {
 	}
 
 	@Test
-	public void testSimpleJdbcInsert() {
+	void usingColumns() {
-		SimpleJdbcInsert jdbcInsert = new SimpleJdbcInsert(dataSource).withTableName("T").usingColumns("F", "S");
+		SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource)
-		jdbcInsert.compile();
+				.withTableName("my_table")
-		String expected = "INSERT INTO T (F, S) VALUES(?, ?)";
+				.usingColumns("col1", "col2");
-		String actual = jdbcInsert.getInsertString();
+
-		assertThat(actual).isEqualTo(expected);
+		insert.compile();
+
+		assertThat(insert.getInsertString()).isEqualTo("INSERT INTO my_table (col1, col2) VALUES(?, ?)");
 	}
 
-	@Test
+	@Test  //  gh-24013
-	public void testSimpleJdbcInsertWithEscapingWithSchemaName() throws Exception {
+	void usingColumnsAndQuotedIdentifiers() throws Exception {
-		SimpleJdbcInsert jdbcInsert = new SimpleJdbcInsert(dataSource).withSchemaName("S").withTableName("T").usingColumns("F", "S").usingEscaping(true);
+		SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource)
+				.withTableName("my_table")
+				.usingColumns("col1", "col2")
+				.usingQuotedIdentifiers();
 
 		given(databaseMetaData.getIdentifierQuoteString()).willReturn("`");
 
-		jdbcInsert.compile();
+		insert.compile();
-		String expected = "INSERT INTO `S.T` (`F`, `S`) VALUES(?, ?)";
+		assertThat(insert.getInsertString()).isEqualTo("INSERT INTO `my_table` (`col1`, `col2`) VALUES(?, ?)");
-		String actual = jdbcInsert.getInsertString();
-		assertThat(actual).isEqualTo(expected);
 	}
 
-	@Test
+	@Test  //  gh-24013
-	public void testSimpleJdbcInsertWithEscapingWithoutSchemaName() throws Exception {
+	void usingColumnsAndQuotedIdentifiersWithSchemaName() throws Exception {
-		SimpleJdbcInsert jdbcInsert = new SimpleJdbcInsert(dataSource).withTableName("T").usingColumns("F", "S").usingEscaping(true);
+		SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource)
+				.withSchemaName("my_schema")
+				.withTableName("my_table")
+				.usingColumns("col1", "col2")
+				.usingQuotedIdentifiers();
 
 		given(databaseMetaData.getIdentifierQuoteString()).willReturn("`");
 
-		jdbcInsert.compile();
+		insert.compile();
-		String expected = "INSERT INTO `T` (`F`, `S`) VALUES(?, ?)";
+		assertThat(insert.getInsertString()).isEqualTo("INSERT INTO `my_schema`.`my_table` (`col1`, `col2`) VALUES(?, ?)");
-		String actual = jdbcInsert.getInsertString();
-		assertThat(actual).isEqualTo(expected);
 	}
 
 }

spring-jdbc/src/test/resources/org/springframework/jdbc/datasource/init/users-schema-with-custom-schema.sql

@@ -0,0 +1,11 @@
+CREATE SCHEMA IF NOT EXISTS my_schema;
+
+SET SCHEMA my_schema;
+
+DROP TABLE users IF EXISTS;
+
+CREATE TABLE users (
+	id INTEGER GENERATED BY DEFAULT AS IDENTITY,
+	first_name VARCHAR(50) NOT NULL,
+	last_name VARCHAR(50) NOT NULL
+);