Support for Id generation in Oracle using quoted identifiers.

The latest Oracle JDBC driver properly supports returning of generated ids, both in batches and for quoted identifiers. This allows us to now support this feature. Closes #1666 Original pull request: #1667
2023-11-15 10:05:46 +01:00
parent cfb22f0565
commit fb8ef0f6e0
10 changed files with 35 additions and 48 deletions
--- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingBatchInsertStrategy.java
+++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingBatchInsertStrategy.java
@@ -64,7 +64,7 @@ class IdGeneratingBatchInsertStrategy implements BatchInsertStrategy {
 		IdGeneration idGeneration = dialect.getIdGeneration();
 		if (idGeneration.driverRequiresKeyColumnNames()) {

-			String[] keyColumnNames = getKeyColumnNames();
+			String[] keyColumnNames = getKeyColumnNames(idGeneration);
 			if (keyColumnNames.length == 0) {
 				batchJdbcOperations.batchUpdate(sql, sqlParameterSources, holder);
 			} else {
@@ -91,10 +91,10 @@ class IdGeneratingBatchInsertStrategy implements BatchInsertStrategy {
 		return ids;
 	}

-	private String[] getKeyColumnNames() {
+	private String[] getKeyColumnNames(IdGeneration idGeneration) {

 		return Optional.ofNullable(idColumn)
-				.map(idColumn -> new String[] { idColumn.getReference() })
+				.map(idColumn -> new String[] {idGeneration.getKeyColumnName( idColumn) })
 				.orElse(new String[0]);
 	}
 }
--- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingInsertStrategy.java
+++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingInsertStrategy.java
@@ -58,7 +58,7 @@ class IdGeneratingInsertStrategy implements InsertStrategy {

 		if (idGeneration.driverRequiresKeyColumnNames()) {

-			String[] keyColumnNames = getKeyColumnNames();
+			String[] keyColumnNames = getKeyColumnNames(idGeneration);
 			if (keyColumnNames.length == 0) {
 				jdbcOperations.update(sql, sqlParameterSource, holder);
 			} else {
@@ -84,8 +84,8 @@ class IdGeneratingInsertStrategy implements InsertStrategy {
 		}
 	}

-	private String[] getKeyColumnNames() {
-		return Optional.ofNullable(idColumn).map(idColumn -> new String[] { idColumn.getReference() })
+	private String[] getKeyColumnNames(IdGeneration idGeneration) {
+		return Optional.ofNullable(idColumn).map(idColumn -> new String[] { idGeneration.getKeyColumnName(idColumn) })
 				.orElse(new String[0]);
 	}
 }
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java
@@ -198,7 +198,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithReferencedEntityById() {

 		template.save(legoSet);
@@ -219,7 +218,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadManyEntitiesWithReferencedEntity() {

 		template.save(legoSet);
@@ -232,7 +230,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-101
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadManyEntitiesWithReferencedEntitySorted() {

 		template.save(createLegoSet("Lava"));
@@ -247,7 +244,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-101
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() {

 		template.save(createLegoSet("Lava"));
@@ -262,7 +258,7 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // GH-821
-	@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, SUPPORTS_NULL_PRECEDENCE })
+	@EnabledOnFeature(SUPPORTS_NULL_PRECEDENCE)
 	void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullPrecedence() {

 		template.save(createLegoSet(null));
@@ -279,7 +275,6 @@ class JdbcAggregateTemplateIntegrationTests {


 	@Test //
-	@EnabledOnFeature({ SUPPORTS_QUOTED_IDS})
 	void findByNonPropertySortFails() {

 		assertThatThrownBy(() -> template.findAll(LegoSet.class,
@@ -289,7 +284,6 @@ class JdbcAggregateTemplateIntegrationTests {


 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadManyEntitiesByIdWithReferencedEntity() {

 		template.save(legoSet);
@@ -301,7 +295,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithReferencedNullEntity() {

 		legoSet.setManual(null);
@@ -314,7 +307,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndDeleteAnEntityWithReferencedEntity() {

 		template.save(legoSet);
@@ -329,7 +321,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndDeleteAllWithReferencedEntity() {

 		template.save(legoSet);
@@ -345,7 +336,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // GH-537
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndDeleteAllByAggregateRootsWithReferencedEntity() {

 		LegoSet legoSet1 = template.save(legoSet);
@@ -362,7 +352,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // GH-537
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndDeleteAllByIdsWithReferencedEntity() {

 		LegoSet legoSet1 = template.save(legoSet);
@@ -423,7 +412,7 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES })
+	@EnabledOnFeature(SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES)
 	void updateReferencedEntityFromNull() {

 		legoSet.setManual(null);
@@ -442,7 +431,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void updateReferencedEntityToNull() {

 		template.save(legoSet);
@@ -473,7 +461,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void replaceReferencedEntity() {

 		template.save(legoSet);
@@ -494,7 +481,7 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-112
-	@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, TestDatabaseFeatures.Feature.SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES })
+	@EnabledOnFeature(TestDatabaseFeatures.Feature.SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES)
 	void changeReferencedEntity() {

 		template.save(legoSet);
@@ -509,7 +496,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-266
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void oneToOneChildWithoutId() {

 		OneToOneParent parent = new OneToOneParent();
@@ -526,7 +512,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-266
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void oneToOneNullChildWithoutId() {

 		OneToOneParent parent = new OneToOneParent();
@@ -542,7 +527,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-266
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void oneToOneNullAttributes() {

 		OneToOneParent parent = new OneToOneParent();
@@ -558,7 +542,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-125
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithSecondaryReferenceNull() {

 		template.save(legoSet);
@@ -571,7 +554,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-125
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithSecondaryReferenceNotNull() {

 		legoSet.alternativeInstructions = new Manual();
@@ -593,7 +575,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-276
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithListOfElementsWithoutId() {

 		ListParent entity = new ListParent();
@@ -612,7 +593,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // GH-498 DATAJDBC-273
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadAnEntityWithListOfElementsInConstructor() {

 		ElementNoId element = new ElementNoId();
@@ -752,7 +732,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-340
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadLongChain() {

 		Chain4 chain4 = new Chain4();
@@ -781,7 +760,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-359
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void saveAndLoadLongChainWithoutIds() {

 		NoIdChain4 chain4 = new NoIdChain4();
@@ -1012,7 +990,6 @@ class JdbcAggregateTemplateIntegrationTests {
 	}

 	@Test // DATAJDBC-462
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	void resavingAnUnversionedEntity() {

 		LegoSet legoSet = new LegoSet();
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateSchemaIntegrationTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateSchemaIntegrationTests.java
@@ -55,7 +55,6 @@ public class JdbcAggregateTemplateSchemaIntegrationTests {
 	@Autowired NamedParameterJdbcOperations jdbcTemplate;

 	@Test
-	@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
 	public void insertFindUpdateDelete() {

 		DummyEntity entity = new DummyEntity();
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/AbstractJdbcRepositoryLookUpStrategyTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/AbstractJdbcRepositoryLookUpStrategyTests.java
@@ -30,7 +30,7 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;

 /**
 * Base class to test <code>@EnableJdbcRepositories(queryLookupStrategy = ...)</code>
- * 
+ *
 * @author Diego Krupitza
 * @since 2.4
 */
--- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java
@@ -40,7 +40,7 @@ public class TestDatabaseFeatures {
 		String productName = jdbcTemplate.execute(
 				(ConnectionCallback<String>) c -> c.getMetaData().getDatabaseProductName().toLowerCase(Locale.ENGLISH));

-		database = Arrays.stream(Database.values()).filter(db -> db.matches(productName)).findFirst().get();
+		database = Arrays.stream(Database.values()).filter(db -> db.matches(productName)).findFirst().orElseThrow();
 	}

 	/**
@@ -50,15 +50,6 @@ public class TestDatabaseFeatures {
 		assumeThat(database).isNotIn(Database.Oracle, Database.SqlServer);
 	}

-	/**
-	 * Oracles JDBC driver seems to have a bug that makes it impossible to acquire generated keys when the column is
-	 * quoted. See
-	 * https://stackoverflow.com/questions/62263576/how-to-get-the-generated-key-for-a-column-with-lowercase-characters-from-oracle
-	 */
-	private void supportsQuotedIds() {
-		assumeThat(database).isNotEqualTo(Database.Oracle);
-	}
-
 	/**
 	 * Microsoft SqlServer does not allow explicitly setting ids in columns where the value gets generated by the
 	 * database. Such columns therefore must not be used in referenced entities, since we do a delete and insert, which
@@ -115,7 +106,6 @@ public class TestDatabaseFeatures {
 	public enum Feature {

 		SUPPORTS_MULTIDIMENSIONAL_ARRAYS(TestDatabaseFeatures::supportsMultiDimensionalArrays), //
-		SUPPORTS_QUOTED_IDS(TestDatabaseFeatures::supportsQuotedIds), //
 		SUPPORTS_HUGE_NUMBERS(TestDatabaseFeatures::supportsHugeNumbers), //
 		SUPPORTS_ARRAYS(TestDatabaseFeatures::supportsArrays), //
 		SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES(TestDatabaseFeatures::supportsGeneratedIdsInReferencedEntities), //
--- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql
+++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql
@@ -43,7 +43,7 @@ CREATE TABLE MANUAL
 (
    "id2"       NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY,
    LEGO_SET    NUMBER,
-    ALTERNATIVE NUMBER,
+    "alternative" NUMBER,
    CONTENT     VARCHAR(2000)
 );

@@ -59,7 +59,7 @@ CREATE TABLE ONE_TO_ONE_PARENT
 CREATE TABLE Child_No_Id
 (
    ONE_TO_ONE_PARENT INTEGER PRIMARY KEY,
-    "content"           VARCHAR(30)
+    CONTENT           VARCHAR(30)
 );

 CREATE TABLE LIST_PARENT
--- a/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateSchemaIntegrationTests-oracle.sql
+++ b/spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateSchemaIntegrationTests-oracle.sql
@@ -1,8 +1,9 @@
 DROP USER OTHER CASCADE;

-
 CREATE USER OTHER;

+ALTER USER OTHER QUOTA UNLIMITED ON USERS;
+
 CREATE TABLE OTHER.DUMMY_ENTITY
 (
  ID  NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY,
--- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java
+++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java
@@ -18,6 +18,8 @@ package org.springframework.data.relational.core.dialect;
 import java.sql.Connection;
 import java.sql.PreparedStatement;

+import org.springframework.data.relational.core.sql.SqlIdentifier;
+
 /**
 * Describes how obtaining generated ids after an insert works for a given JDBC driver.
 *
@@ -45,6 +47,18 @@ public interface IdGeneration {
 		return false;
 	}

+	/**
+	 * Provides for a given id {@link SqlIdentifier} the String that is to be used for registering interest in the
+	 * generated value of that column.
+	 * 
+	 * @param id {@link SqlIdentifier} representing a column for which a generated value is to be obtained.
+	 * @return a String representing that column in the way expected by the JDBC driver.
+	 * @since 3.3
+	 */
+	default String getKeyColumnName(SqlIdentifier id) {
+		return id.getReference();
+	}
+
 	/**
 	 * Does the driver support id generation for batch operations.
 	 * <p>
--- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java
+++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java
@@ -18,6 +18,7 @@ package org.springframework.data.relational.core.dialect;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.convert.ReadingConverter;
 import org.springframework.data.convert.WritingConverter;
+import org.springframework.data.relational.core.sql.SqlIdentifier;

 import java.util.Collection;
 import java.util.Collections;
@@ -42,6 +43,11 @@ public class OracleDialect extends AnsiDialect {
 		public boolean driverRequiresKeyColumnNames() {
 			return true;
 		}
+
+		@Override
+		public String getKeyColumnName(SqlIdentifier id) {
+			return id.toSql(INSTANCE.getIdentifierProcessing());
+		}
 	};

 	protected OracleDialect() {}