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 f07a322e3c
commit 2be10dd5fd
10 changed files with 37 additions and 47 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
@@ -67,7 +67,7 @@ class IdGeneratingBatchInsertStrategy implements BatchInsertStrategy {
 		IdGeneration idGeneration = dialect.getIdGeneration();
 		if (idGeneration.driverRequiresKeyColumnNames()) {

-			String[] keyColumnNames = getKeyColumnNames();
+			String[] keyColumnNames = getKeyColumnNames(idGeneration);
 			if (keyColumnNames.length == 0) {
 				jdbcOperations.batchUpdate(sql, sqlParameterSources, holder);
 			} else {
@@ -94,10 +94,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/AbstractJdbcAggregateTemplateIntegrationTests.java
+++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java
@@ -286,7 +286,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -304,7 +303,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -317,7 +315,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(createLegoSet("Lava"));
@@ -332,7 +329,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(createLegoSet("Lava"));
@@ -347,7 +343,7 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(createLegoSet(null));
@@ -363,7 +359,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		assertThatThrownBy(() -> template.findAll(LegoSet.class, Sort.by("somethingNotExistant")))
@@ -371,7 +366,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -383,7 +377,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		legoSet.manual = null;
@@ -396,7 +389,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -408,7 +400,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -420,7 +411,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		LegoSet legoSet1 = template.save(legoSet);
@@ -434,7 +424,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		LegoSet legoSet1 = template.save(legoSet);
@@ -494,7 +483,7 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		legoSet.manual = (null);
@@ -513,7 +502,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -541,7 +529,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -559,7 +546,7 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

 	@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);
@@ -574,7 +561,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		OneToOneParent parent = new OneToOneParent();
@@ -591,7 +577,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		OneToOneParent parent = new OneToOneParent();
@@ -607,7 +592,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		OneToOneParent parent = new OneToOneParent();
@@ -623,7 +607,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		template.save(legoSet);
@@ -636,7 +619,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		legoSet.alternativeInstructions = new Manual();
@@ -655,7 +637,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		ListParent entity = new ListParent();
@@ -674,7 +655,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		ElementNoId element = new ElementNoId();
@@ -815,7 +795,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		Chain4 chain4 = new Chain4();
@@ -844,7 +823,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

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

 		NoIdChain4 chain4 = new NoIdChain4();
@@ -1084,7 +1062,6 @@ abstract class AbstractJdbcAggregateTemplateIntegrationTests {
 	}

 	@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
@@ -46,7 +46,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
@@ -24,6 +24,8 @@ import java.util.stream.Collectors;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.annotation.Id;
 import org.springframework.data.jdbc.repository.query.Query;
+import org.springframework.data.jdbc.testing.DatabaseType;
+import org.springframework.data.jdbc.testing.EnabledOnDatabase;
 import org.springframework.data.relational.core.mapping.RelationalMappingContext;
 import org.springframework.data.repository.CrudRepository;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
@@ -34,6 +36,7 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 * @author Diego Krupitza
 * @since 2.4
 */
+@EnabledOnDatabase(DatabaseType.HSQL)
 abstract class AbstractJdbcRepositoryLookUpStrategyTests {

 	@Autowired protected OnesRepository onesRepository;
--- 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
@@ -51,7 +51,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)
 );

@@ -67,7 +67,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
@@ -17,6 +17,7 @@ package org.springframework.data.relational.core.dialect;

 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.convert.WritingConverter;
+import org.springframework.data.relational.core.sql.SqlIdentifier;

 import java.util.Collection;

@@ -40,6 +41,11 @@ public class OracleDialect extends AnsiDialect {
 		public boolean driverRequiresKeyColumnNames() {
 			return true;
 		}
+
+		@Override
+		public String getKeyColumnName(SqlIdentifier id) {
+			return id.toSql(INSTANCE.getIdentifierProcessing());
+		}
 	};

 	protected OracleDialect() {}