Skip to content

Commit 30961bf

Browse files
schaudermp911de
schauder
authored and
mp911de
committed
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
1 parent 9828b2e commit 30961bf

File tree

10 files changed

+37
-47
lines changed

10 files changed

+37
-47
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingBatchInsertStrategy.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public Object[] execute(String sql, SqlParameterSource[] sqlParameterSources) {
6767
IdGeneration idGeneration = dialect.getIdGeneration();
6868
if (idGeneration.driverRequiresKeyColumnNames()) {
6969

70-
String[] keyColumnNames = getKeyColumnNames();
70+
String[] keyColumnNames = getKeyColumnNames(idGeneration);
7171
if (keyColumnNames.length == 0) {
7272
jdbcOperations.batchUpdate(sql, sqlParameterSources, holder);
7373
} else {
@@ -94,10 +94,10 @@ public Object[] execute(String sql, SqlParameterSource[] sqlParameterSources) {
9494
return ids;
9595
}
9696

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

9999
return Optional.ofNullable(idColumn)
100-
.map(idColumn -> new String[] { idColumn.getReference() })
100+
.map(idColumn -> new String[] {idGeneration.getKeyColumnName( idColumn) })
101101
.orElse(new String[0]);
102102
}
103103
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/IdGeneratingInsertStrategy.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public Object execute(String sql, SqlParameterSource sqlParameterSource) {
5858

5959
if (idGeneration.driverRequiresKeyColumnNames()) {
6060

61-
String[] keyColumnNames = getKeyColumnNames();
61+
String[] keyColumnNames = getKeyColumnNames(idGeneration);
6262
if (keyColumnNames.length == 0) {
6363
jdbcOperations.update(sql, sqlParameterSource, holder);
6464
} else {
@@ -84,8 +84,8 @@ public Object execute(String sql, SqlParameterSource sqlParameterSource) {
8484
}
8585
}
8686

87-
private String[] getKeyColumnNames() {
88-
return Optional.ofNullable(idColumn).map(idColumn -> new String[] { idColumn.getReference() })
87+
private String[] getKeyColumnNames(IdGeneration idGeneration) {
88+
return Optional.ofNullable(idColumn).map(idColumn -> new String[] { idGeneration.getKeyColumnName(idColumn) })
8989
.orElse(new String[0]);
9090
}
9191
}

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

+3-26
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@ void findOneByQueryToManyResults() {
286286
}
287287

288288
@Test // DATAJDBC-112
289-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
290289
void saveAndLoadAnEntityWithReferencedEntityById() {
291290

292291
template.save(legoSet);
@@ -304,7 +303,6 @@ void saveAndLoadAnEntityWithReferencedEntityById() {
304303
}
305304

306305
@Test // DATAJDBC-112
307-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
308306
void saveAndLoadManyEntitiesWithReferencedEntity() {
309307

310308
template.save(legoSet);
@@ -317,7 +315,6 @@ void saveAndLoadManyEntitiesWithReferencedEntity() {
317315
}
318316

319317
@Test // DATAJDBC-101
320-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
321318
void saveAndLoadManyEntitiesWithReferencedEntitySorted() {
322319

323320
template.save(createLegoSet("Lava"));
@@ -332,7 +329,6 @@ void saveAndLoadManyEntitiesWithReferencedEntitySorted() {
332329
}
333330

334331
@Test // DATAJDBC-101
335-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
336332
void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() {
337333

338334
template.save(createLegoSet("Lava"));
@@ -347,7 +343,7 @@ void saveAndLoadManyEntitiesWithReferencedEntitySortedAndPaged() {
347343
}
348344

349345
@Test // GH-821
350-
@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, SUPPORTS_NULL_PRECEDENCE })
346+
@EnabledOnFeature(SUPPORTS_NULL_PRECEDENCE)
351347
void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullPrecedence() {
352348

353349
template.save(createLegoSet(null));
@@ -363,15 +359,13 @@ void saveAndLoadManyEntitiesWithReferencedEntitySortedWithNullPrecedence() {
363359
}
364360

365361
@Test //
366-
@EnabledOnFeature({ SUPPORTS_QUOTED_IDS })
367362
void findByNonPropertySortFails() {
368363

369364
assertThatThrownBy(() -> template.findAll(LegoSet.class, Sort.by("somethingNotExistant")))
370365
.isInstanceOf(InvalidPersistentPropertyPath.class);
371366
}
372367

373368
@Test // DATAJDBC-112
374-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
375369
void saveAndLoadManyEntitiesByIdWithReferencedEntity() {
376370

377371
template.save(legoSet);
@@ -383,7 +377,6 @@ void saveAndLoadManyEntitiesByIdWithReferencedEntity() {
383377
}
384378

385379
@Test // DATAJDBC-112
386-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
387380
void saveAndLoadAnEntityWithReferencedNullEntity() {
388381

389382
legoSet.manual = null;
@@ -396,7 +389,6 @@ void saveAndLoadAnEntityWithReferencedNullEntity() {
396389
}
397390

398391
@Test // DATAJDBC-112
399-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
400392
void saveAndDeleteAnEntityWithReferencedEntity() {
401393

402394
template.save(legoSet);
@@ -408,7 +400,6 @@ void saveAndDeleteAnEntityWithReferencedEntity() {
408400
}
409401

410402
@Test // DATAJDBC-112
411-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
412403
void saveAndDeleteAllWithReferencedEntity() {
413404

414405
template.save(legoSet);
@@ -420,7 +411,6 @@ void saveAndDeleteAllWithReferencedEntity() {
420411
}
421412

422413
@Test // GH-537
423-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
424414
void saveAndDeleteAllByAggregateRootsWithReferencedEntity() {
425415

426416
LegoSet legoSet1 = template.save(legoSet);
@@ -434,7 +424,6 @@ void saveAndDeleteAllByAggregateRootsWithReferencedEntity() {
434424
}
435425

436426
@Test // GH-537
437-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
438427
void saveAndDeleteAllByIdsWithReferencedEntity() {
439428

440429
LegoSet legoSet1 = template.save(legoSet);
@@ -494,7 +483,7 @@ void insertAndUpdateAllByAggregateRootsWithVersion() {
494483
}
495484

496485
@Test // DATAJDBC-112
497-
@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES })
486+
@EnabledOnFeature(SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES)
498487
void updateReferencedEntityFromNull() {
499488

500489
legoSet.manual = (null);
@@ -513,7 +502,6 @@ void updateReferencedEntityFromNull() {
513502
}
514503

515504
@Test // DATAJDBC-112
516-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
517505
void updateReferencedEntityToNull() {
518506

519507
template.save(legoSet);
@@ -541,7 +529,6 @@ void updateFailedRootDoesNotExist() {
541529
}
542530

543531
@Test // DATAJDBC-112
544-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
545532
void replaceReferencedEntity() {
546533

547534
template.save(legoSet);
@@ -559,7 +546,7 @@ void replaceReferencedEntity() {
559546
}
560547

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

565552
template.save(legoSet);
@@ -574,7 +561,6 @@ void changeReferencedEntity() {
574561
}
575562

576563
@Test // DATAJDBC-266
577-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
578564
void oneToOneChildWithoutId() {
579565

580566
OneToOneParent parent = new OneToOneParent();
@@ -591,7 +577,6 @@ void oneToOneChildWithoutId() {
591577
}
592578

593579
@Test // DATAJDBC-266
594-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
595580
void oneToOneNullChildWithoutId() {
596581

597582
OneToOneParent parent = new OneToOneParent();
@@ -607,7 +592,6 @@ void oneToOneNullChildWithoutId() {
607592
}
608593

609594
@Test // DATAJDBC-266
610-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
611595
void oneToOneNullAttributes() {
612596

613597
OneToOneParent parent = new OneToOneParent();
@@ -623,7 +607,6 @@ void oneToOneNullAttributes() {
623607
}
624608

625609
@Test // DATAJDBC-125
626-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
627610
void saveAndLoadAnEntityWithSecondaryReferenceNull() {
628611

629612
template.save(legoSet);
@@ -636,7 +619,6 @@ void saveAndLoadAnEntityWithSecondaryReferenceNull() {
636619
}
637620

638621
@Test // DATAJDBC-125
639-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
640622
void saveAndLoadAnEntityWithSecondaryReferenceNotNull() {
641623

642624
legoSet.alternativeInstructions = new Manual();
@@ -655,7 +637,6 @@ void saveAndLoadAnEntityWithSecondaryReferenceNotNull() {
655637
}
656638

657639
@Test // DATAJDBC-276
658-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
659640
void saveAndLoadAnEntityWithListOfElementsWithoutId() {
660641

661642
ListParent entity = new ListParent();
@@ -674,7 +655,6 @@ void saveAndLoadAnEntityWithListOfElementsWithoutId() {
674655
}
675656

676657
@Test // GH-498 DATAJDBC-273
677-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
678658
void saveAndLoadAnEntityWithListOfElementsInConstructor() {
679659

680660
ElementNoId element = new ElementNoId();
@@ -815,7 +795,6 @@ void saveAndLoadAnEntityWithByteArray() {
815795
}
816796

817797
@Test // DATAJDBC-340
818-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
819798
void saveAndLoadLongChain() {
820799

821800
Chain4 chain4 = new Chain4();
@@ -844,7 +823,6 @@ void saveAndLoadLongChain() {
844823
}
845824

846825
@Test // DATAJDBC-359
847-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
848826
void saveAndLoadLongChainWithoutIds() {
849827

850828
NoIdChain4 chain4 = new NoIdChain4();
@@ -1084,7 +1062,6 @@ void saveAndUpdateAggregateWithIdAndNullVersion() {
10841062
}
10851063

10861064
@Test // DATAJDBC-462
1087-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
10881065
void resavingAnUnversionedEntity() {
10891066

10901067
LegoSet legoSet = new LegoSet();

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateSchemaIntegrationTests.java

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ public class JdbcAggregateTemplateSchemaIntegrationTests {
4646
@Autowired NamedParameterJdbcOperations jdbcTemplate;
4747

4848
@Test
49-
@EnabledOnFeature(SUPPORTS_QUOTED_IDS)
5049
public void insertFindUpdateDelete() {
5150

5251
DummyEntity entity = new DummyEntity();

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/AbstractJdbcRepositoryLookUpStrategyTests.java

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.springframework.beans.factory.annotation.Autowired;
2525
import org.springframework.data.annotation.Id;
2626
import org.springframework.data.jdbc.repository.query.Query;
27+
import org.springframework.data.jdbc.testing.DatabaseType;
28+
import org.springframework.data.jdbc.testing.EnabledOnDatabase;
2729
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
2830
import org.springframework.data.repository.CrudRepository;
2931
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
@@ -34,6 +36,7 @@
3436
* @author Diego Krupitza
3537
* @since 2.4
3638
*/
39+
@EnabledOnDatabase(DatabaseType.HSQL)
3740
abstract class AbstractJdbcRepositoryLookUpStrategyTests {
3841

3942
@Autowired protected OnesRepository onesRepository;

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/testing/TestDatabaseFeatures.java

+1-11
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public TestDatabaseFeatures(JdbcOperations jdbcTemplate) {
4040
String productName = jdbcTemplate.execute(
4141
(ConnectionCallback<String>) c -> c.getMetaData().getDatabaseProductName().toLowerCase(Locale.ENGLISH));
4242

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

4646
/**
@@ -50,15 +50,6 @@ private void supportsHugeNumbers() {
5050
assumeThat(database).isNotIn(Database.Oracle, Database.SqlServer);
5151
}
5252

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

117108
SUPPORTS_MULTIDIMENSIONAL_ARRAYS(TestDatabaseFeatures::supportsMultiDimensionalArrays), //
118-
SUPPORTS_QUOTED_IDS(TestDatabaseFeatures::supportsQuotedIds), //
119109
SUPPORTS_HUGE_NUMBERS(TestDatabaseFeatures::supportsHugeNumbers), //
120110
SUPPORTS_ARRAYS(TestDatabaseFeatures::supportsArrays), //
121111
SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES(TestDatabaseFeatures::supportsGeneratedIdsInReferencedEntities), //

‎spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateIntegrationTests-oracle.sql

+2-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ CREATE TABLE MANUAL
5151
(
5252
"id2" NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY,
5353
LEGO_SET NUMBER,
54-
ALTERNATIVE NUMBER,
54+
"alternative" NUMBER,
5555
CONTENT VARCHAR(2000)
5656
);
5757

@@ -67,7 +67,7 @@ CREATE TABLE ONE_TO_ONE_PARENT
6767
CREATE TABLE Child_No_Id
6868
(
6969
ONE_TO_ONE_PARENT INTEGER PRIMARY KEY,
70-
"content" VARCHAR(30)
70+
CONTENT VARCHAR(30)
7171
);
7272

7373
CREATE TABLE LIST_PARENT

‎spring-data-jdbc/src/test/resources/org.springframework.data.jdbc.core/JdbcAggregateTemplateSchemaIntegrationTests-oracle.sql

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
DROP USER OTHER CASCADE;
22

3-
43
CREATE USER OTHER;
54

5+
ALTER USER OTHER QUOTA UNLIMITED ON USERS;
6+
67
CREATE TABLE OTHER.DUMMY_ENTITY
78
(
89
ID NUMBER GENERATED by default on null as IDENTITY PRIMARY KEY,

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java

+14
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import java.sql.Connection;
1919
import java.sql.PreparedStatement;
2020

21+
import org.springframework.data.relational.core.sql.SqlIdentifier;
22+
2123
/**
2224
* Describes how obtaining generated ids after an insert works for a given JDBC driver.
2325
*
@@ -45,6 +47,18 @@ default boolean driverRequiresKeyColumnNames() {
4547
return false;
4648
}
4749

50+
/**
51+
* Provides for a given id {@link SqlIdentifier} the String that is to be used for registering interest in the
52+
* generated value of that column.
53+
*
54+
* @param id {@link SqlIdentifier} representing a column for which a generated value is to be obtained.
55+
* @return a String representing that column in the way expected by the JDBC driver.
56+
* @since 3.3
57+
*/
58+
default String getKeyColumnName(SqlIdentifier id) {
59+
return id.getReference();
60+
}
61+
4862
/**
4963
* Does the driver support id generation for batch operations.
5064
* <p>

‎spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java

+6
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import org.springframework.core.convert.converter.Converter;
1919
import org.springframework.data.convert.WritingConverter;
20+
import org.springframework.data.relational.core.sql.SqlIdentifier;
2021

2122
import java.util.Collection;
2223

@@ -40,6 +41,11 @@ public class OracleDialect extends AnsiDialect {
4041
public boolean driverRequiresKeyColumnNames() {
4142
return true;
4243
}
44+
45+
@Override
46+
public String getKeyColumnName(SqlIdentifier id) {
47+
return id.toSql(INSTANCE.getIdentifierProcessing());
48+
}
4349
};
4450

4551
protected OracleDialect() {}

0 commit comments

Comments
 (0)