From 42048361b743949258db868d3197169598e08fdc Mon Sep 17 00:00:00 2001 From: radovanradic Date: Tue, 19 May 2026 16:25:45 +0200 Subject: [PATCH 01/20] Fix @OneToOne composite @JoinColumn handling for @EmbeddedId in JDBC --- .../OneToOneEmbeddedIdJoinColumnSpec.groovy | 131 ++++++++++++ .../data/model/naming/NamingStrategy.java | 20 ++ .../query/builder/sql/SqlQueryBuilder.java | 193 +++++++++--------- 3 files changed, 246 insertions(+), 98 deletions(-) create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy new file mode 100644 index 0000000000..2cfa5d1286 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy @@ -0,0 +1,131 @@ +package io.micronaut.data.jdbc.issue3415 + +import io.micronaut.data.annotation.Embeddable +import io.micronaut.data.annotation.EmbeddedId +import io.micronaut.data.annotation.Join +import io.micronaut.data.annotation.MappedEntity +import io.micronaut.data.annotation.MappedProperty +import io.micronaut.data.annotation.Relation +import io.micronaut.data.jdbc.annotation.JdbcRepository +import io.micronaut.data.jdbc.h2.H2DBProperties +import io.micronaut.data.model.query.builder.sql.Dialect +import io.micronaut.data.repository.CrudRepository +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import jakarta.persistence.JoinColumn +import spock.lang.Shared +import spock.lang.Specification + +import java.sql.Connection +import java.util.UUID + +@MicronautTest +@H2DBProperties(packages = "io.micronaut.data.jdbc.issue3415", schemaGenerate = "NONE") +class OneToOneEmbeddedIdJoinColumnSpec extends Specification { + + @Shared + @Inject + AssetRepository assetRepository + + @Shared + @Inject + Connection connection + + void setup() { + try (def s = connection.createStatement()) { + s.execute(''' +DROP TABLE IF EXISTS asset; +DROP TABLE IF EXISTS assetmetadata; + +CREATE TABLE asset ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + title VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +); + +CREATE TABLE assetmetadata ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + author VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +); +''') + } + } + + void 'save owning one-to-one with composite join columns and embedded id'() { + given: + def id = new AssetId(containerId: UUID.randomUUID(), assetId: 1) + + when: + assetRepository.save(new Asset(id: id, title: 'title')) + def saved = assetRepository.findById(id).orElse(null) + + then: + saved != null + saved.id.containerId == id.containerId + saved.id.assetId == id.assetId + saved.title == 'title' + } + + void 'fetch join owning one-to-one with composite join columns and embedded id'() { + given: + def id = new AssetId(containerId: UUID.fromString('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8'), assetId: 1) + try (def s = connection.createStatement()) { + s.execute(""" +INSERT INTO assetmetadata (container_id, asset_id, author) VALUES ('${id.containerId}', ${id.assetId}, 'chris'); +INSERT INTO asset (container_id, asset_id, title) VALUES ('${id.containerId}', ${id.assetId}, 'Llama Llama'); +""") + } + + when: + def asset = assetRepository.findById(id).orElse(null) + + then: + asset != null + asset.metadata != null + asset.metadata.author == 'chris' + } +} + +@JdbcRepository(dialect = Dialect.H2) +interface AssetRepository extends CrudRepository { + + @Join(value = "metadata", type = Join.Type.LEFT_FETCH) + @Override + Optional findById(AssetId id) +} + +@Embeddable +class AssetId { + + @MappedProperty("container_id") + UUID containerId + + @MappedProperty("asset_id") + Integer assetId +} + +@MappedEntity("asset") +class Asset { + + @EmbeddedId + AssetId id + + String title + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "container_id", referencedColumnName = "container_id") + @JoinColumn(name = "asset_id", referencedColumnName = "asset_id") + AssetMetadata metadata +} + +@MappedEntity("assetmetadata") +class AssetMetadata { + + @EmbeddedId + AssetId id + + String author +} diff --git a/data-model/src/main/java/io/micronaut/data/model/naming/NamingStrategy.java b/data-model/src/main/java/io/micronaut/data/model/naming/NamingStrategy.java index 2e1a104bf9..827fdbeff1 100644 --- a/data-model/src/main/java/io/micronaut/data/model/naming/NamingStrategy.java +++ b/data-model/src/main/java/io/micronaut/data/model/naming/NamingStrategy.java @@ -26,10 +26,12 @@ import io.micronaut.data.annotation.MappedProperty; import io.micronaut.data.annotation.Projection; import io.micronaut.data.annotation.Relation; +import io.micronaut.data.annotation.sql.JoinColumns; import io.micronaut.data.model.Association; import io.micronaut.data.model.Embedded; import io.micronaut.data.model.PersistentEntity; import io.micronaut.data.model.PersistentProperty; +import org.jspecify.annotations.Nullable; import java.util.List; import java.util.Optional; @@ -175,6 +177,10 @@ default String mappedName(List associations, PersistentProperty p } } if (foreignAssociation != null) { + String joinColumnName = resolveJoinColumnName(foreignAssociation, property); + if (joinColumnName != null) { + return joinColumnName; + } PersistentEntity associatedEntity = foreignAssociation.getAssociatedEntity(); if (associatedEntity.equals(property.getOwner()) && associatedEntity.hasIdentity() && associatedEntity.getIdentity().equals(property)) { String providedName = foreignAssociation.getAnnotationMetadata().stringValue(MappedProperty.class).orElse(null); @@ -200,6 +206,20 @@ default String mappedName(List associations, PersistentProperty p return mappedName(sb.toString()); } + private static @Nullable String resolveJoinColumnName(Association association, PersistentProperty property) { + AnnotationValue joinColumns = association.getAnnotationMetadata().getAnnotation(JoinColumns.class); + if (joinColumns == null) { + return null; + } + for (AnnotationValue joinColumn : joinColumns.getAnnotations(AnnotationMetadata.VALUE_MEMBER)) { + String referencedColumnName = joinColumn.stringValue("referencedColumnName").orElse(null); + if (referencedColumnName != null && referencedColumnName.equals(property.getPersistedName())) { + return joinColumn.stringValue("name").orElse(null); + } + } + return null; + } + default String mappedJoinTableColumn(PersistentEntity associated, List associations, PersistentProperty property) { StringBuilder sb = new StringBuilder(); sb.append(associated.getDecapitalizedName()); diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index a6615404ba..1970f91c62 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1220,56 +1220,34 @@ public JsonDataType getJsonDataType() { for (PersistentProperty prop : persistentProperties) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), prop, (associations, property) -> { boolean generated = SqlQueryBuilderUtils.isGeneratedProperty(property, associations); + String unescapedColumnName = getMappedName(namingStrategy, associations, property); + String columnName = unescapedColumnName; + if (escape) { + columnName = quote(columnName); + } if (generated) { - String columnName = getMappedName(namingStrategy, associations, property); - unescapedColumns.add(columnName); - if (escape) { - columnName = quote(columnName); - } + unescapedColumns.add(unescapedColumnName); resultColumns.add(columnName); resultColumnTypes.add(property.getDataType()); return; } - addWriteExpression(values, property); - - String key = String.valueOf(values.size()); String[] path = asStringPath(associations, property); - parameterBindings.add(new QueryParameterBinding() { - @Override - public String getName() { - return key; - } - - @Override - public String getKey() { - return key; - } - - @Override - public DataType getDataType() { - return property.getDataType(); - } - - @Override - public JsonDataType getJsonDataType() { - return property.getJsonDataType(); - } - - @Override - public String[] getPropertyPath() { - return path; - } - }); - - String columnName = getMappedName(namingStrategy, associations, property); - unescapedColumns.add(columnName); - if (escape) { - columnName = quote(columnName); + int existingIndex = columns.indexOf(columnName); + String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); + QueryParameterBinding binding = createInsertParameterBinding(key, property, path); + String valueExpression = getWriteExpression(Integer.parseInt(key), property); + if (existingIndex == -1) { + values.add(valueExpression); + parameterBindings.add(binding); + unescapedColumns.add(unescapedColumnName); + columns.add(columnName); + resultColumns.add(columnName); + resultColumnTypes.add(property.getDataType()); + } else { + values.set(existingIndex, valueExpression); + parameterBindings.set(existingIndex, binding); } - columns.add(columnName); - resultColumns.add(columnName); - resultColumnTypes.add(property.getDataType()); }); } if (entity.hasVersion()) { @@ -1320,13 +1298,15 @@ public String[] getPropertyPath() { if (escape) { columnName = quote(columnName); } + int existingIndex = columns.indexOf(columnName); boolean isSequence = false; if (SqlQueryBuilderUtils.isNotForeign(associations)) { - - unescapedColumns.add(unescapedColumnName); - resultColumns.add(columnName); - resultColumnTypes.add(property.getDataType()); + if (existingIndex == -1) { + unescapedColumns.add(unescapedColumnName); + resultColumns.add(columnName); + resultColumnTypes.add(property.getDataType()); + } Optional> generated = property.findAnnotation(GeneratedValue.class); if (generated.isPresent()) { @@ -1343,43 +1323,29 @@ public String[] getPropertyPath() { } if (isSequence) { - values.add(getSequenceStatement(unescapedSchema, unescapedTableName, property)); + String sequenceStatement = getSequenceStatement(unescapedSchema, unescapedTableName, property); + if (existingIndex == -1) { + values.add(sequenceStatement); + } else { + values.set(existingIndex, sequenceStatement); + } } else { - addWriteExpression(values, property); - - String key = String.valueOf(values.size()); String[] path = asStringPath(associations, property); - parameterBindings.add(new QueryParameterBinding() { - - @Override - public String getName() { - return key; - } - - @Override - public String getKey() { - return key; - } - - @Override - public DataType getDataType() { - return property.getDataType(); - } - - @Override - public JsonDataType getJsonDataType() { - return property.getJsonDataType(); - } - - @Override - public String[] getPropertyPath() { - return path; - } - }); - + String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); + QueryParameterBinding binding = createInsertParameterBinding(key, property, path); + String valueExpression = getWriteExpression(Integer.parseInt(key), property); + if (existingIndex == -1) { + values.add(valueExpression); + parameterBindings.add(binding); + } else { + values.set(existingIndex, valueExpression); + parameterBindings.set(existingIndex, binding); + } } - columns.add(columnName); + if (existingIndex == -1) { + columns.add(columnName); + } }); } @@ -1490,33 +1456,64 @@ private String getObjectName(@Nullable String schema, String objectName, boolean } } + private QueryParameterBinding createInsertParameterBinding(String key, PersistentProperty property, String[] path) { + return new QueryParameterBinding() { + @Override + public String getName() { + return key; + } + + @Override + public String getKey() { + return key; + } + + @Override + public DataType getDataType() { + return property.getDataType(); + } + + @Override + public JsonDataType getJsonDataType() { + return property.getJsonDataType(); + } + + @Override + public String[] getPropertyPath() { + return path; + } + }; + } + private boolean addWriteExpression(List values, PersistentProperty property) { + return values.add(getWriteExpression(values.size() + 1, property)); + } + + private String getWriteExpression(int parameterIndex, PersistentProperty property) { DataType dt = property.getDataType(); String transformer = getDataTransformerWriteValue(null, property).orElse(null); if (transformer != null) { - return values.add(transformer); + return transformer; } - String param = formatParameter(values.size() + 1).name(); + String param = formatParameter(parameterIndex).name(); if (dt == DataType.JSON) { - switch (dialect) { - case POSTGRES -> values.add("to_json(" + param + "::json)"); - case H2 -> values.add(param + " FORMAT JSON"); - case MYSQL -> values.add("CONVERT(" + param + " USING UTF8MB4)"); - default -> values.add(param); - } - return true; + return switch (dialect) { + case POSTGRES -> "to_json(" + param + "::json)"; + case H2 -> param + " FORMAT JSON"; + case MYSQL -> "CONVERT(" + param + " USING UTF8MB4)"; + default -> param; + }; } if (isJsonOrWktGeometry(property)) { - switch (dialect) { - case ORACLE -> values.add(getOracleGeometryExpression(param, property)); - case MYSQL -> values.add(getMysqlGeometryExpression(param, property)); - case SQL_SERVER -> values.add(getSqlServerGeometryExpression(param, property)); - case POSTGRES, H2 -> values.add(getPostgresGeometryExpression(param, property)); - default -> values.add(param); - } - return true; - } - return values.add(param); + return switch (dialect) { + case ORACLE -> getOracleGeometryExpression(param, property); + case MYSQL -> getMysqlGeometryExpression(param, property); + case SQL_SERVER -> getSqlServerGeometryExpression(param, property); + case POSTGRES, H2 -> getPostgresGeometryExpression(param, property); + default -> param; + }; + } + return param; } @Override From 2bcf3455da69d22d9ad90663febf39c60807254c Mon Sep 17 00:00:00 2001 From: radovanradic Date: Tue, 19 May 2026 16:25:54 +0200 Subject: [PATCH 02/20] Trigger build --- .github/workflows/graalvm-latest.yml | 1 + .github/workflows/gradle.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/graalvm-latest.yml b/.github/workflows/graalvm-latest.yml index e35dfe3b38..f377b4b4a2 100644 --- a/.github/workflows/graalvm-latest.yml +++ b/.github/workflows/graalvm-latest.yml @@ -9,6 +9,7 @@ on: branches: - master - '[0-9]+.[0-9]+.x' + - issue-3415 pull_request: branches: - master diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 63af75b3a3..83adb838cf 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -9,6 +9,7 @@ on: branches: - master - '[0-9]+.[0-9]+.x' + - issue-3415 pull_request: branches: - master From 38fe3502970e746cdd4fc8dcccf6a45d266b64ad Mon Sep 17 00:00:00 2001 From: radovanradic Date: Tue, 19 May 2026 16:43:08 +0200 Subject: [PATCH 03/20] Fix @OneToOne composite @JoinColumn handling for @EmbeddedId in JDBC --- ...sOneToOneSharedSequenceIdentitySpec.groovy | 93 ++++++++++++ .../query/builder/sql/SqlQueryBuilder.java | 135 +++++++++++++++++- .../data/processor/sql/BuildInsertSpec.groovy | 56 ++++++++ 3 files changed, 278 insertions(+), 6 deletions(-) create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy new file mode 100644 index 0000000000..3b8a4a59e9 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy @@ -0,0 +1,93 @@ +package io.micronaut.data.jdbc.postgres.issue3415 + +import io.micronaut.data.annotation.GeneratedValue +import io.micronaut.data.annotation.Id +import io.micronaut.data.annotation.MappedEntity +import io.micronaut.data.annotation.Relation +import io.micronaut.data.jdbc.annotation.JdbcRepository +import io.micronaut.data.jdbc.postgres.PostgresTestPropertyProvider +import io.micronaut.data.model.query.builder.sql.Dialect +import io.micronaut.data.runtime.config.SchemaGenerate +import io.micronaut.data.repository.CrudRepository +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import jakarta.persistence.JoinColumn +import spock.lang.Specification + +import java.sql.Connection + +@MicronautTest +class PostgresOneToOneSharedSequenceIdentitySpec extends Specification implements PostgresTestPropertyProvider { + + @Inject + Connection connection + + @Inject + SharedSequenceAssetRepository sharedSequenceAssetRepository + + @Override + List packages() { + return List.of("io.micronaut.data.jdbc.postgres.issue3415") + } + + @Override + SchemaGenerate schemaGenerate() { + return SchemaGenerate.NONE + } + + void setup() { + connection.prepareStatement(''' +DROP TABLE IF EXISTS sequence_assetmetadata; +DROP TABLE IF EXISTS sequence_asset; +DROP SEQUENCE IF EXISTS sequence_asset_seq; + +CREATE SEQUENCE sequence_asset_seq START WITH 1 INCREMENT BY 1; + +CREATE TABLE sequence_asset ( + id BIGINT NOT NULL PRIMARY KEY, + title VARCHAR(255) +); + +CREATE TABLE sequence_assetmetadata ( + id BIGINT NOT NULL PRIMARY KEY, + author VARCHAR(255) +); +''').withCloseable { it.executeUpdate() } + } + + void 'save shared-key one-to-one with sequence identity reuses the physical id column'() { + when: + def saved = sharedSequenceAssetRepository.save(new SharedSequenceAsset(title: 'title')) + + then: + saved.id == 1L + sharedSequenceAssetRepository.findById(saved.id).orElse(null)?.title == 'title' + } +} + +@JdbcRepository(dialect = Dialect.POSTGRES) +interface SharedSequenceAssetRepository extends CrudRepository { +} + +@MappedEntity("sequence_asset") +class SharedSequenceAsset { + + @Id + @GeneratedValue(value = GeneratedValue.Type.SEQUENCE, ref = "sequence_asset_seq") + Long id + + String title + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "id", referencedColumnName = "id") + SharedSequenceAssetMetadata metadata +} + +@MappedEntity("sequence_assetmetadata") +class SharedSequenceAssetMetadata { + + @Id + Long id + + String author +} diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 1970f91c62..c368d5b2db 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1216,6 +1216,7 @@ public JsonDataType getJsonDataType() { Collection persistentProperties = entity.getPersistentProperties(); List columns = new ArrayList<>(); List values = new ArrayList<>(); + List<@Nullable QueryParameterBinding> valueBindings = new ArrayList<>(); for (PersistentProperty prop : persistentProperties) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), prop, (associations, property) -> { @@ -1234,19 +1235,23 @@ public JsonDataType getJsonDataType() { String[] path = asStringPath(associations, property); int existingIndex = columns.indexOf(columnName); + if (existingIndex != -1) { + QueryParameterBinding existingBinding = valueBindings.get(existingIndex); + failOnConflictingInsertColumn(entity, unescapedColumnName, existingBinding == null ? null : existingBinding.getPropertyPath(), path); + } String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); QueryParameterBinding binding = createInsertParameterBinding(key, property, path); String valueExpression = getWriteExpression(Integer.parseInt(key), property); if (existingIndex == -1) { values.add(valueExpression); - parameterBindings.add(binding); + valueBindings.add(binding); unescapedColumns.add(unescapedColumnName); columns.add(columnName); resultColumns.add(columnName); resultColumnTypes.add(property.getDataType()); } else { values.set(existingIndex, valueExpression); - parameterBindings.set(existingIndex, binding); + valueBindings.set(existingIndex, binding); } }); } @@ -1256,7 +1261,7 @@ public JsonDataType getJsonDataType() { addWriteExpression(values, version); String key = String.valueOf(values.size()); - parameterBindings.add(new QueryParameterBinding() { + valueBindings.add(new QueryParameterBinding() { @Override public String getName() { @@ -1298,7 +1303,15 @@ public String[] getPropertyPath() { if (escape) { columnName = quote(columnName); } + String[] path = asStringPath(associations, property); int existingIndex = columns.indexOf(columnName); + if (existingIndex != -1) { + QueryParameterBinding existingBinding = valueBindings.get(existingIndex); + String @Nullable [] existingPath = existingBinding == null ? null : existingBinding.getPropertyPath(); + if (!isAllowedSharedIdentityColumnReuse(existingPath, path)) { + failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); + } + } boolean isSequence = false; if (SqlQueryBuilderUtils.isNotForeign(associations)) { @@ -1326,20 +1339,21 @@ public String[] getPropertyPath() { String sequenceStatement = getSequenceStatement(unescapedSchema, unescapedTableName, property); if (existingIndex == -1) { values.add(sequenceStatement); + valueBindings.add(null); } else { values.set(existingIndex, sequenceStatement); + valueBindings.set(existingIndex, null); } } else { - String[] path = asStringPath(associations, property); String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); QueryParameterBinding binding = createInsertParameterBinding(key, property, path); String valueExpression = getWriteExpression(Integer.parseInt(key), property); if (existingIndex == -1) { values.add(valueExpression); - parameterBindings.add(binding); + valueBindings.add(binding); } else { values.set(existingIndex, valueExpression); - parameterBindings.set(existingIndex, binding); + valueBindings.set(existingIndex, binding); } } @@ -1349,6 +1363,8 @@ public String[] getPropertyPath() { }); } + parameterBindings = compactInsertParameterBindings(valueBindings); + builder = INSERT_INTO + getTableName(entity) + " (" + String.join(",", columns) + CLOSE_BRACKET + " " + "VALUES (" + String.join(String.valueOf(COMMA), values) + CLOSE_BRACKET; @@ -1396,6 +1412,113 @@ public DataType getDataType() { Collections.emptyMap()); } + private void failOnConflictingInsertColumn(PersistentEntity entity, String columnName, String @Nullable [] existingPath, String[] path) { + throw new MappingException("Conflicting insert mapping for column [" + columnName + "] on entity [" + entity.getName() + "] between paths " + + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); + } + + private List compactInsertParameterBindings(List<@Nullable QueryParameterBinding> valueBindings) { + List bindings = new ArrayList<>(valueBindings.size()); + int bindingIndex = 1; + for (QueryParameterBinding binding : valueBindings) { + if (binding != null) { + bindings.add(reindexInsertParameterBinding(String.valueOf(bindingIndex++), binding)); + } + } + return bindings; + } + + private QueryParameterBinding reindexInsertParameterBinding(String key, QueryParameterBinding delegate) { + return new QueryParameterBinding() { + @Override + public String getName() { + return key; + } + + @Override + public String getKey() { + return key; + } + + @Override + public DataType getDataType() { + return delegate.getDataType(); + } + + @Override + public JsonDataType getJsonDataType() { + return delegate.getJsonDataType(); + } + + @Override + public @Nullable String getConverterClassName() { + return delegate.getConverterClassName(); + } + + @Override + public int getParameterIndex() { + return delegate.getParameterIndex(); + } + + @Override + public String @Nullable [] getParameterBindingPath() { + return delegate.getParameterBindingPath(); + } + + @Override + public String @Nullable [] getPropertyPath() { + return delegate.getPropertyPath(); + } + + @Override + public boolean isAutoPopulated() { + return delegate.isAutoPopulated(); + } + + @Override + public boolean isRequiresPreviousPopulatedValue() { + return delegate.isRequiresPreviousPopulatedValue(); + } + + @Override + public boolean isExpandable() { + return delegate.isExpandable(); + } + + @Override + public @Nullable Object getValue() { + return delegate.getValue(); + } + + @Override + public boolean isExpression() { + return delegate.isExpression(); + } + + @Override + public @Nullable String getRole() { + return delegate.getRole(); + } + + @Override + public @Nullable String getTableAlias() { + return delegate.getTableAlias(); + } + }; + } + + private boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] existingPath, String[] identityPath) { + if (existingPath == null || existingPath.length <= identityPath.length) { + return false; + } + for (int i = 1; i <= identityPath.length; i++) { + if (!Objects.equals(existingPath[existingPath.length - i], identityPath[identityPath.length - i])) { + return false; + } + } + return true; + } + private String[] asStringPath(List associations, PersistentProperty property) { if (associations.isEmpty()) { return new String[]{property.getName()}; diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index 82bed2ee54..715156617f 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -901,4 +901,60 @@ interface BookRepository extends GenericRepository { ex.message.contains("must declare explicit returned columns instead of RETURNING *") } + void "duplicate physical insert columns fail fast unless reused for shared identity"() { + when: + buildRepository('test.ConflictingInsertEntityRepository', """ +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; + +@JdbcRepository(dialect = Dialect.H2) +@io.micronaut.context.annotation.Executable +interface ConflictingInsertEntityRepository extends CrudRepository { +} + +@MappedEntity("conflicting_insert_entity") +class ConflictingInsertEntity { + @Id + private Long id; + + @MappedProperty("shared_value") + private String firstValue; + + @MappedProperty("shared_value") + private String secondValue; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getFirstValue() { + return firstValue; + } + + void setFirstValue(String firstValue) { + this.firstValue = firstValue; + } + + String getSecondValue() { + return secondValue; + } + + void setSecondValue(String secondValue) { + this.secondValue = secondValue; + } +} +""") + + then: + def ex = thrown(RuntimeException) + ex.message.contains("Conflicting insert mapping for column [shared_value]") + } + } From 71e57ad6582aa4b7240c153785d077ce5fd730c8 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Tue, 19 May 2026 16:48:42 +0200 Subject: [PATCH 04/20] Fix @OneToOne composite @JoinColumn handling for @EmbeddedId in JDBC --- .../one2one}/OneToOneEmbeddedIdJoinColumnSpec.groovy | 4 ++-- .../PostgresOneToOneSharedSequenceIdentitySpec.groovy | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename data-jdbc/src/test/groovy/io/micronaut/data/jdbc/{issue3415 => h2/one2one}/OneToOneEmbeddedIdJoinColumnSpec.groovy (96%) rename data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/{issue3415 => one2one}/PostgresOneToOneSharedSequenceIdentitySpec.groovy (95%) diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy similarity index 96% rename from data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy rename to data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy index 2cfa5d1286..028a8fb244 100644 --- a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/issue3415/OneToOneEmbeddedIdJoinColumnSpec.groovy +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy @@ -1,4 +1,4 @@ -package io.micronaut.data.jdbc.issue3415 +package io.micronaut.data.jdbc.h2.one2one import io.micronaut.data.annotation.Embeddable import io.micronaut.data.annotation.EmbeddedId @@ -20,7 +20,7 @@ import java.sql.Connection import java.util.UUID @MicronautTest -@H2DBProperties(packages = "io.micronaut.data.jdbc.issue3415", schemaGenerate = "NONE") +@H2DBProperties(packages = "io.micronaut.data.jdbc.h2.one2one", schemaGenerate = "NONE") class OneToOneEmbeddedIdJoinColumnSpec extends Specification { @Shared diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy similarity index 95% rename from data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy rename to data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy index 3b8a4a59e9..08bf2934aa 100644 --- a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/issue3415/PostgresOneToOneSharedSequenceIdentitySpec.groovy +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy @@ -1,4 +1,4 @@ -package io.micronaut.data.jdbc.postgres.issue3415 +package io.micronaut.data.jdbc.postgres.one2one import io.micronaut.data.annotation.GeneratedValue import io.micronaut.data.annotation.Id @@ -27,7 +27,7 @@ class PostgresOneToOneSharedSequenceIdentitySpec extends Specification implement @Override List packages() { - return List.of("io.micronaut.data.jdbc.postgres.issue3415") + return List.of("io.micronaut.data.jdbc.postgres.one2one") } @Override From a3cd680adae6cbe055e1005be50e61614c34c969 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 11:23:07 +0200 Subject: [PATCH 05/20] Fix shared-key @OneToOne insert/DDL column reuse for embedded and sequence identities, including Postgres R2DBC placeholder numbering. --- .../query/builder/sql/SqlQueryBuilder.java | 210 ++++++------------ .../query/builder/sql/SqlSchemaUtils.java | 69 +++++- .../data/processor/sql/BuildInsertSpec.groovy | 115 ++++++++++ .../data/processor/sql/BuildTableSpec.groovy | 101 +++++++++ ...sOneToOneSharedSequenceIdentitySpec.groovy | 136 ++++++++++++ 5 files changed, 485 insertions(+), 146 deletions(-) create mode 100644 data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index c368d5b2db..2876260a47 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1215,8 +1215,7 @@ public JsonDataType getJsonDataType() { Collection persistentProperties = entity.getPersistentProperties(); List columns = new ArrayList<>(); - List values = new ArrayList<>(); - List<@Nullable QueryParameterBinding> valueBindings = new ArrayList<>(); + List valueSlots = new ArrayList<>(); for (PersistentProperty prop : persistentProperties) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), prop, (associations, property) -> { @@ -1236,53 +1235,24 @@ public JsonDataType getJsonDataType() { String[] path = asStringPath(associations, property); int existingIndex = columns.indexOf(columnName); if (existingIndex != -1) { - QueryParameterBinding existingBinding = valueBindings.get(existingIndex); - failOnConflictingInsertColumn(entity, unescapedColumnName, existingBinding == null ? null : existingBinding.getPropertyPath(), path); + InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); + failOnConflictingInsertColumn(entity, unescapedColumnName, existingValueSlot.getPropertyPath(), path); } - String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); - QueryParameterBinding binding = createInsertParameterBinding(key, property, path); - String valueExpression = getWriteExpression(Integer.parseInt(key), property); if (existingIndex == -1) { - values.add(valueExpression); - valueBindings.add(binding); + valueSlots.add(InsertValueSlot.binding(property, path)); unescapedColumns.add(unescapedColumnName); columns.add(columnName); resultColumns.add(columnName); resultColumnTypes.add(property.getDataType()); } else { - values.set(existingIndex, valueExpression); - valueBindings.set(existingIndex, binding); + valueSlots.set(existingIndex, InsertValueSlot.binding(property, path)); } }); } if (entity.hasVersion()) { PersistentProperty version = entity.getVersion(); if (!version.isGenerated()) { - addWriteExpression(values, version); - - String key = String.valueOf(values.size()); - valueBindings.add(new QueryParameterBinding() { - - @Override - public String getName() { - return key; - } - - @Override - public String getKey() { - return key; - } - - @Override - public DataType getDataType() { - return version.getDataType(); - } - - @Override - public String[] getPropertyPath() { - return new String[]{version.getName()}; - } - }); + valueSlots.add(InsertValueSlot.binding(version, new String[]{version.getName()})); String columnName = getMappedName(namingStrategy, Collections.emptyList(), version); unescapedColumns.add(columnName); @@ -1306,8 +1276,8 @@ public String[] getPropertyPath() { String[] path = asStringPath(associations, property); int existingIndex = columns.indexOf(columnName); if (existingIndex != -1) { - QueryParameterBinding existingBinding = valueBindings.get(existingIndex); - String @Nullable [] existingPath = existingBinding == null ? null : existingBinding.getPropertyPath(); + InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); + String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); if (!isAllowedSharedIdentityColumnReuse(existingPath, path)) { failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); } @@ -1338,22 +1308,15 @@ public String[] getPropertyPath() { if (isSequence) { String sequenceStatement = getSequenceStatement(unescapedSchema, unescapedTableName, property); if (existingIndex == -1) { - values.add(sequenceStatement); - valueBindings.add(null); + valueSlots.add(InsertValueSlot.literal(sequenceStatement)); } else { - values.set(existingIndex, sequenceStatement); - valueBindings.set(existingIndex, null); + valueSlots.set(existingIndex, InsertValueSlot.literal(sequenceStatement)); } } else { - String key = String.valueOf(existingIndex == -1 ? values.size() + 1 : existingIndex + 1); - QueryParameterBinding binding = createInsertParameterBinding(key, property, path); - String valueExpression = getWriteExpression(Integer.parseInt(key), property); if (existingIndex == -1) { - values.add(valueExpression); - valueBindings.add(binding); + valueSlots.add(InsertValueSlot.binding(property, path)); } else { - values.set(existingIndex, valueExpression); - valueBindings.set(existingIndex, binding); + valueSlots.set(existingIndex, InsertValueSlot.binding(property, path)); } } @@ -1363,7 +1326,18 @@ public String[] getPropertyPath() { }); } - parameterBindings = compactInsertParameterBindings(valueBindings); + List values = new ArrayList<>(valueSlots.size()); + parameterBindings = new ArrayList<>(valueSlots.size()); + int bindingIndex = 1; + for (InsertValueSlot valueSlot : valueSlots) { + if (valueSlot.isLiteral()) { + values.add(valueSlot.getFixedExpression()); + continue; + } + values.add(getWriteExpression(bindingIndex, valueSlot.getProperty())); + parameterBindings.add(createInsertParameterBinding(String.valueOf(bindingIndex), valueSlot.getProperty(), valueSlot.getRequiredPropertyPath())); + bindingIndex++; + } builder = INSERT_INTO + getTableName(entity) + " (" + String.join(",", columns) + CLOSE_BRACKET + " " + @@ -1417,96 +1391,6 @@ private void failOnConflictingInsertColumn(PersistentEntity entity, String colum + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); } - private List compactInsertParameterBindings(List<@Nullable QueryParameterBinding> valueBindings) { - List bindings = new ArrayList<>(valueBindings.size()); - int bindingIndex = 1; - for (QueryParameterBinding binding : valueBindings) { - if (binding != null) { - bindings.add(reindexInsertParameterBinding(String.valueOf(bindingIndex++), binding)); - } - } - return bindings; - } - - private QueryParameterBinding reindexInsertParameterBinding(String key, QueryParameterBinding delegate) { - return new QueryParameterBinding() { - @Override - public String getName() { - return key; - } - - @Override - public String getKey() { - return key; - } - - @Override - public DataType getDataType() { - return delegate.getDataType(); - } - - @Override - public JsonDataType getJsonDataType() { - return delegate.getJsonDataType(); - } - - @Override - public @Nullable String getConverterClassName() { - return delegate.getConverterClassName(); - } - - @Override - public int getParameterIndex() { - return delegate.getParameterIndex(); - } - - @Override - public String @Nullable [] getParameterBindingPath() { - return delegate.getParameterBindingPath(); - } - - @Override - public String @Nullable [] getPropertyPath() { - return delegate.getPropertyPath(); - } - - @Override - public boolean isAutoPopulated() { - return delegate.isAutoPopulated(); - } - - @Override - public boolean isRequiresPreviousPopulatedValue() { - return delegate.isRequiresPreviousPopulatedValue(); - } - - @Override - public boolean isExpandable() { - return delegate.isExpandable(); - } - - @Override - public @Nullable Object getValue() { - return delegate.getValue(); - } - - @Override - public boolean isExpression() { - return delegate.isExpression(); - } - - @Override - public @Nullable String getRole() { - return delegate.getRole(); - } - - @Override - public @Nullable String getTableAlias() { - return delegate.getTableAlias(); - } - }; - } - private boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] existingPath, String[] identityPath) { if (existingPath == null || existingPath.length <= identityPath.length) { return false; @@ -1608,10 +1492,6 @@ public String[] getPropertyPath() { }; } - private boolean addWriteExpression(List values, PersistentProperty property) { - return values.add(getWriteExpression(values.size() + 1, property)); - } - private String getWriteExpression(int parameterIndex, PersistentProperty property) { DataType dt = property.getDataType(); String transformer = getDataTransformerWriteValue(null, property).orElse(null); @@ -1639,6 +1519,48 @@ private String getWriteExpression(int parameterIndex, PersistentProperty propert return param; } + private static final class InsertValueSlot { + @Nullable + private final PersistentProperty property; + private final String @Nullable [] propertyPath; + @Nullable + private final String fixedExpression; + + private InsertValueSlot(@Nullable PersistentProperty property, String @Nullable [] propertyPath, @Nullable String fixedExpression) { + this.property = property; + this.propertyPath = propertyPath; + this.fixedExpression = fixedExpression; + } + + private static InsertValueSlot binding(PersistentProperty property, String[] propertyPath) { + return new InsertValueSlot(property, propertyPath, null); + } + + private static InsertValueSlot literal(String fixedExpression) { + return new InsertValueSlot(null, null, fixedExpression); + } + + private boolean isLiteral() { + return fixedExpression != null; + } + + private PersistentProperty getProperty() { + return Objects.requireNonNull(property); + } + + private String @Nullable [] getPropertyPath() { + return propertyPath; + } + + private String[] getRequiredPropertyPath() { + return Objects.requireNonNull(propertyPath); + } + + private String getFixedExpression() { + return Objects.requireNonNull(fixedExpression); + } + } + @Override protected void appendUpdateSetParameter(StringBuilder sb, @Nullable String alias, PersistentProperty prop, Runnable appendParameter) { String transformed = getDataTransformerWriteValue(alias, prop).orElse(null); diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index 870cd476e8..1682783a41 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -65,6 +65,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -205,13 +206,23 @@ public static List getSqlTableMappings(List List primaryKeyColumns = getPrimaryKeyColumns(sqlColumnDefinitionProviders, identities, namingStrategy, dialect); List columns = new ArrayList<>(); + Map identityColumnPaths = new LinkedHashMap<>(); + Map columnPaths = new LinkedHashMap<>(); + for (PersistentProperty identity : identities) { + PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { + String columnName = namingStrategy.mappedName(associations, property); + String[] path = asStringPath(associations, property); + identityColumnPaths.putIfAbsent(columnName, path); + columnPaths.putIfAbsent(columnName, path); + }); + } if (entity.hasVersion()) { PersistentProperty version = entity.getVersion(); if (!version.isGenerated()) { String columnName = namingStrategy.mappedName(Collections.emptyList(), version); SqlColumnMapping column = getColumnDefinition(sqlColumnDefinitionProviders, version, columnName, false, true, false, dialect); - columns.add(column); + addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, new String[]{version.getName()}, column); } } @@ -219,7 +230,7 @@ public static List getSqlTableMappings(List String columnName = namingStrategy.mappedName(associations, property); SqlColumnMapping column = getColumnDefinition(sqlColumnDefinitionProviders, property, columnName, false, isRequired(associations, property), !SqlQueryBuilderUtils.isNotForeign(associations), dialect); - columns.add(column); + addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, asStringPath(associations, property), column); }; for (PersistentProperty prop : entity.getPersistentProperties()) { @@ -236,6 +247,60 @@ public static List getSqlTableMappings(List return tables; } + private static void addTableColumn(PersistentEntity entity, + List columns, + Map columnPaths, + Map identityColumnPaths, + String columnName, + String[] path, + SqlColumnMapping column) { + String[] existingPath = columnPaths.get(columnName); + if (existingPath != null) { + String @Nullable [] identityPath = identityColumnPaths.get(columnName); + if (Arrays.equals(existingPath, path) || isAllowedSharedIdentityColumnReuse(identityPath, existingPath, path)) { + return; + } + throw new MappingException("Conflicting table mapping for column [" + columnName + "] on entity [" + entity.getName() + "] between paths " + + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); + } + columnPaths.put(columnName, path); + columns.add(column); + } + + private static boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] identityPath, + String[] existingPath, + String[] newPath) { + if (identityPath == null) { + return false; + } + return (Arrays.equals(existingPath, identityPath) && hasMatchingSuffix(newPath, identityPath)) + || (Arrays.equals(newPath, identityPath) && hasMatchingSuffix(existingPath, identityPath)); + } + + private static boolean hasMatchingSuffix(String[] longerPath, String[] shorterPath) { + if (longerPath.length <= shorterPath.length) { + return false; + } + for (int i = 1; i <= shorterPath.length; i++) { + if (!longerPath[longerPath.length - i].equals(shorterPath[shorterPath.length - i])) { + return false; + } + } + return true; + } + + private static String[] asStringPath(List associations, PersistentProperty property) { + if (associations.isEmpty()) { + return new String[]{property.getName()}; + } + List path = new ArrayList<>(associations.size() + 1); + for (Association association : associations) { + path.add(association.getName()); + } + path.add(property.getName()); + return path.toArray(new String[0]); + } + /** * Create Join table columns. * diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index 715156617f..5b15cfd7fd 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -957,4 +957,119 @@ class ConflictingInsertEntity { ex.message.contains("Conflicting insert mapping for column [shared_value]") } + void "shared identity sequence insert keeps contiguous postgres placeholders for r2dbc"() { + given: + BeanDefinition beanDefinition = buildRepository('test.SharedSequenceAssetRepository', """ +import io.micronaut.context.annotation.AliasFor; +import io.micronaut.data.annotation.Repository; +import io.micronaut.data.annotation.RepositoryConfiguration; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.model.query.builder.sql.SqlQueryBuilder; +import io.micronaut.data.model.query.builder.sql.SqlQueryConfiguration; +import jakarta.persistence.JoinColumn; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@RepositoryConfiguration(queryBuilder = SqlQueryBuilder.class, implicitQueries = false, namedParameters = false) +@SqlQueryConfiguration( + @SqlQueryConfiguration.DialectConfiguration( + dialect = Dialect.POSTGRES, + positionalParameterFormat = "\$%s" + ) +) +@Retention(RetentionPolicy.RUNTIME) +@Repository +@interface TestPostgresRepository { + @AliasFor(annotation = Repository.class, member = "dialect") + Dialect dialect() default Dialect.ANSI; +} + +@TestPostgresRepository(dialect = Dialect.POSTGRES) +@io.micronaut.context.annotation.Executable +interface SharedSequenceAssetRepository extends CrudRepository { +} + +@MappedEntity("sequence_asset") +class SharedSequenceAsset { + @Id + @GeneratedValue(value = GeneratedValue.Type.SEQUENCE, ref = "sequence_asset_seq") + private Long id; + + private String title; + + @Version + private Long version; + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "id", referencedColumnName = "id") + private SharedSequenceAssetMetadata metadata; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getTitle() { + return title; + } + + void setTitle(String title) { + this.title = title; + } + + Long getVersion() { + return version; + } + + void setVersion(Long version) { + this.version = version; + } + + SharedSequenceAssetMetadata getMetadata() { + return metadata; + } + + void setMetadata(SharedSequenceAssetMetadata metadata) { + this.metadata = metadata; + } +} + +@MappedEntity("sequence_assetmetadata") +class SharedSequenceAssetMetadata { + @Id + private Long id; + + private String author; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getAuthor() { + return author; + } + + void setAuthor(String author) { + this.author = author; + } +} +""") + + def method = beanDefinition.findPossibleMethods("save") + .toList() + .find { it.arguments.length == 1 && it.arguments[0].type.name == 'test.SharedSequenceAsset' } + + expect: + method != null + getQuery(method) == 'INSERT INTO "sequence_asset" ("title","id","version") VALUES ($1,nextval(\'sequence_asset_seq\'),$2)' + getParameterPropertyPaths(method) == ["title", "version"] as String[] + } + } diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy index a1b4f5361f..be8d08c028 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy @@ -40,6 +40,107 @@ class BuildTableSpec extends AbstractDataSpec { sql.contains("\"street\" VARCHAR(255) NOT NULL,") } + void "shared identity join columns do not duplicate embedded id columns in ddl"() { + given: + def entity = buildEntity('test.Asset', ''' +import java.util.UUID; +import jakarta.persistence.JoinColumn; + +@MappedEntity("asset") +class Asset { + @EmbeddedId + private AssetId id; + + private String title; + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "container_id", referencedColumnName = "container_id") + @JoinColumn(name = "asset_id", referencedColumnName = "asset_id") + private AssetMetadata metadata; + + AssetId getId() { + return id; + } + + void setId(AssetId id) { + this.id = id; + } + + String getTitle() { + return title; + } + + void setTitle(String title) { + this.title = title; + } + + AssetMetadata getMetadata() { + return metadata; + } + + void setMetadata(AssetMetadata metadata) { + this.metadata = metadata; + } +} + +@Embeddable +class AssetId { + @MappedProperty("container_id") + private UUID containerId; + + @MappedProperty("asset_id") + private Integer assetId; + + UUID getContainerId() { + return containerId; + } + + void setContainerId(UUID containerId) { + this.containerId = containerId; + } + + Integer getAssetId() { + return assetId; + } + + void setAssetId(Integer assetId) { + this.assetId = assetId; + } +} + +@MappedEntity("assetmetadata") +class AssetMetadata { + @EmbeddedId + private AssetId id; + + private String author; + + AssetId getId() { + return id; + } + + void setId(AssetId id) { + this.id = id; + } + + String getAuthor() { + return author; + } + + void setAuthor(String author) { + this.author = author; + } +} +''') + SqlQueryBuilder builder = new SqlQueryBuilder(Dialect.H2) + def sql = builder.buildBatchCreateTableStatement(List.of(), entity) + + expect: + !sql.contains("metadata") + sql.count("`container_id`") == 2 + sql.count("`asset_id`") == 2 + } + @Unroll void "test build create table for JSON type for dialect #dialect"() { given: diff --git a/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy b/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy new file mode 100644 index 0000000000..71e1229681 --- /dev/null +++ b/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneSharedSequenceIdentitySpec.groovy @@ -0,0 +1,136 @@ +package io.micronaut.data.r2dbc.postgres.one2one + +import io.micronaut.context.ApplicationContext +import io.micronaut.data.annotation.GeneratedValue +import io.micronaut.data.annotation.Id +import io.micronaut.data.annotation.Join +import io.micronaut.data.annotation.MappedEntity +import io.micronaut.data.annotation.Relation +import io.micronaut.data.annotation.Version +import io.micronaut.data.model.query.builder.sql.Dialect +import io.micronaut.data.r2dbc.annotation.R2dbcRepository +import io.micronaut.data.r2dbc.postgres.PostgresTestPropertyProvider +import io.micronaut.data.repository.reactive.ReactorCrudRepository +import io.micronaut.data.runtime.config.SchemaGenerate +import io.r2dbc.spi.Connection +import io.r2dbc.spi.ConnectionFactory +import io.r2dbc.spi.Result +import jakarta.persistence.JoinColumn +import reactor.core.publisher.Flux +import reactor.core.publisher.Mono +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +class PostgresOneToOneSharedSequenceIdentitySpec extends Specification implements PostgresTestPropertyProvider { + + @AutoCleanup + @Shared + ApplicationContext context = ApplicationContext.run(properties) + + @Shared + ConnectionFactory connectionFactory = context.getBean(ConnectionFactory) + + @Shared + SharedSequenceAssetRepository sharedSequenceAssetRepository = context.getBean(SharedSequenceAssetRepository) + + @Override + SchemaGenerate schemaGenerate() { + return SchemaGenerate.NONE + } + + void setup() { + executeStatements([ + 'DROP TABLE IF EXISTS sequence_assetmetadata', + 'DROP TABLE IF EXISTS sequence_asset', + 'DROP SEQUENCE IF EXISTS sequence_asset_seq', + 'CREATE SEQUENCE sequence_asset_seq START WITH 1 INCREMENT BY 1', + '''CREATE TABLE sequence_asset ( + id BIGINT NOT NULL PRIMARY KEY, + title VARCHAR(255), + version BIGINT NOT NULL +)''', + '''CREATE TABLE sequence_assetmetadata ( + id BIGINT NOT NULL PRIMARY KEY, + author VARCHAR(255) +)''' + ]) + } + + void 'save shared-key one-to-one with sequence identity reuses the physical id column'() { + when: + def saved = sharedSequenceAssetRepository.save(new SharedSequenceAsset(title: 'title')).block() + + then: + saved != null + saved.id == 1L + saved.version != null + sharedSequenceAssetRepository.findById(saved.id).block()?.title == 'title' + } + + void 'fetch join shared-key one-to-one uses the shared id column'() { + given: + executeStatements([ + "INSERT INTO sequence_assetmetadata (id, author) VALUES (7, 'chris')", + "INSERT INTO sequence_asset (id, title, version) VALUES (7, 'title', 0)" + ]) + + when: + def asset = sharedSequenceAssetRepository.findById(7L).block() + + then: + asset != null + asset.metadata != null + asset.metadata.author == 'chris' + } + + private void executeStatements(List statements) { + Mono.usingWhen( + Mono.from(connectionFactory.create()), + { Connection connection -> + Flux.fromIterable(statements) + .concatMap { String sql -> + Flux.from(connection.createStatement(sql).execute()) + .flatMap(Result::getRowsUpdated) + .then() + } + .then() + }, + Connection::close + ).block() + } +} + +@R2dbcRepository(dialect = Dialect.POSTGRES) +interface SharedSequenceAssetRepository extends ReactorCrudRepository { + + @Join(value = 'metadata', type = Join.Type.LEFT_FETCH) + @Override + Mono findById(Long id) +} + +@MappedEntity("sequence_asset") +class SharedSequenceAsset { + + @Id + @GeneratedValue(value = GeneratedValue.Type.SEQUENCE, ref = "sequence_asset_seq") + Long id + + String title + + @Version + Long version + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "id", referencedColumnName = "id") + SharedSequenceAssetMetadata metadata +} + +@MappedEntity("sequence_assetmetadata") +class SharedSequenceAssetMetadata { + + @Id + Long id + + String author +} From 9a214c0513f3420697a5902e19bd940e5dee34ce Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 12:54:43 +0200 Subject: [PATCH 06/20] Fix checkstyle errors. --- .../query/builder/sql/SqlQueryBuilder.java | 85 +++++++++---------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 2876260a47..7c5a0b9817 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -66,7 +66,6 @@ import io.micronaut.data.model.query.builder.QueryResult; import io.micronaut.data.model.runtime.convert.SqlIndexDefinitionProvider; import io.micronaut.data.model.schema.sql.SqlColumnMapping; -import io.micronaut.data.model.schema.sql.SqlDbType; import io.micronaut.data.model.schema.sql.SqlIndexMapping; import io.micronaut.data.model.schema.sql.SqlSequenceMapping; import io.micronaut.data.model.schema.sql.SqlTableMapping; @@ -1519,48 +1518,6 @@ private String getWriteExpression(int parameterIndex, PersistentProperty propert return param; } - private static final class InsertValueSlot { - @Nullable - private final PersistentProperty property; - private final String @Nullable [] propertyPath; - @Nullable - private final String fixedExpression; - - private InsertValueSlot(@Nullable PersistentProperty property, String @Nullable [] propertyPath, @Nullable String fixedExpression) { - this.property = property; - this.propertyPath = propertyPath; - this.fixedExpression = fixedExpression; - } - - private static InsertValueSlot binding(PersistentProperty property, String[] propertyPath) { - return new InsertValueSlot(property, propertyPath, null); - } - - private static InsertValueSlot literal(String fixedExpression) { - return new InsertValueSlot(null, null, fixedExpression); - } - - private boolean isLiteral() { - return fixedExpression != null; - } - - private PersistentProperty getProperty() { - return Objects.requireNonNull(property); - } - - private String @Nullable [] getPropertyPath() { - return propertyPath; - } - - private String[] getRequiredPropertyPath() { - return Objects.requireNonNull(propertyPath); - } - - private String getFixedExpression() { - return Objects.requireNonNull(fixedExpression); - } - } - @Override protected void appendUpdateSetParameter(StringBuilder sb, @Nullable String alias, PersistentProperty prop, Runnable appendParameter) { String transformed = getDataTransformerWriteValue(alias, prop).orElse(null); @@ -2495,4 +2452,46 @@ protected void appendPropertyProjection(QueryPropertyPath propertyPath) { } } + private static final class InsertValueSlot { + @Nullable + private final PersistentProperty property; + private final String @Nullable [] propertyPath; + @Nullable + private final String fixedExpression; + + private InsertValueSlot(@Nullable PersistentProperty property, String @Nullable [] propertyPath, @Nullable String fixedExpression) { + this.property = property; + this.propertyPath = propertyPath; + this.fixedExpression = fixedExpression; + } + + private static InsertValueSlot binding(PersistentProperty property, String[] propertyPath) { + return new InsertValueSlot(property, propertyPath, null); + } + + private static InsertValueSlot literal(String fixedExpression) { + return new InsertValueSlot(null, null, fixedExpression); + } + + private boolean isLiteral() { + return fixedExpression != null; + } + + private PersistentProperty getProperty() { + return Objects.requireNonNull(property); + } + + private String @Nullable [] getPropertyPath() { + return propertyPath; + } + + private String[] getRequiredPropertyPath() { + return Objects.requireNonNull(propertyPath); + } + + private String getFixedExpression() { + return Objects.requireNonNull(fixedExpression); + } + } + } From fbd13822392d0ca7c0318e2a179e506a2579c5a5 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 14:00:31 +0200 Subject: [PATCH 07/20] Fix fetch-join projection for one-to-one @JoinColumns with embedded IDs and add record-based JDBC regression coverage --- ...ToOneEmbeddedIdJoinColumnRecordSpec.groovy | 60 +++++++++++++++++++ .../data/jdbc/h2/one2one/RecordAsset.java | 17 ++++++ .../data/jdbc/h2/one2one/RecordAssetId.java | 13 ++++ .../jdbc/h2/one2one/RecordAssetMetadata.java | 11 ++++ .../h2/one2one/RecordAssetRepository.java | 16 +++++ .../query/builder/sql/SqlQueryBuilder.java | 11 +++- 6 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnRecordSpec.groovy create mode 100644 data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAsset.java create mode 100644 data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetId.java create mode 100644 data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetMetadata.java create mode 100644 data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetRepository.java diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnRecordSpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnRecordSpec.groovy new file mode 100644 index 0000000000..1e454c42c9 --- /dev/null +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnRecordSpec.groovy @@ -0,0 +1,60 @@ +package io.micronaut.data.jdbc.h2.one2one + +import io.micronaut.data.jdbc.h2.H2DBProperties +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import spock.lang.Specification + +import java.sql.Connection +import java.util.UUID + +@MicronautTest +@H2DBProperties(packages = "io.micronaut.data.jdbc.h2.one2one", schemaGenerate = "NONE") +class OneToOneEmbeddedIdJoinColumnRecordSpec extends Specification { + + @Inject + RecordAssetRepository recordAssetRepository + + @Inject + Connection connection + + void setup() { + try (def s = connection.createStatement()) { + s.execute(''' +DROP TABLE IF EXISTS record_asset; +DROP TABLE IF EXISTS record_assetmetadata; + +CREATE TABLE record_asset ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + title VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +); + +CREATE TABLE record_assetmetadata ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + author VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +); + +INSERT INTO record_assetmetadata (container_id, asset_id, author) VALUES ('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8', 1, 'chris'); +INSERT INTO record_asset (container_id, asset_id, title) VALUES ('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8', 1, 'Llama Llama'); +''') + } + } + + void 'fetch join owning one-to-one with composite join columns and embedded id records'() { + given: + def id = new RecordAssetId(UUID.fromString('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8'), 1) + + when: + def asset = recordAssetRepository.findById(id).orElse(null) + + then: + asset != null + asset.metadata() != null + asset.metadata().author() == 'chris' + asset.metadata().id() == id + } +} diff --git a/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAsset.java b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAsset.java new file mode 100644 index 0000000000..0d11cc3017 --- /dev/null +++ b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAsset.java @@ -0,0 +1,17 @@ +package io.micronaut.data.jdbc.h2.one2one; + +import io.micronaut.data.annotation.EmbeddedId; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.Relation; +import jakarta.persistence.JoinColumn; + +@MappedEntity("record_asset") +public record RecordAsset( + @EmbeddedId RecordAssetId id, + String title, + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "container_id", referencedColumnName = "container_id") + @JoinColumn(name = "asset_id", referencedColumnName = "asset_id") + RecordAssetMetadata metadata +) { +} diff --git a/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetId.java b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetId.java new file mode 100644 index 0000000000..9cbbf6116f --- /dev/null +++ b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetId.java @@ -0,0 +1,13 @@ +package io.micronaut.data.jdbc.h2.one2one; + +import io.micronaut.data.annotation.Embeddable; +import io.micronaut.data.annotation.MappedProperty; + +import java.util.UUID; + +@Embeddable +public record RecordAssetId( + @MappedProperty("container_id") UUID containerId, + @MappedProperty("asset_id") Integer assetId +) { +} diff --git a/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetMetadata.java b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetMetadata.java new file mode 100644 index 0000000000..34ae5eeda5 --- /dev/null +++ b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetMetadata.java @@ -0,0 +1,11 @@ +package io.micronaut.data.jdbc.h2.one2one; + +import io.micronaut.data.annotation.EmbeddedId; +import io.micronaut.data.annotation.MappedEntity; + +@MappedEntity("record_assetmetadata") +public record RecordAssetMetadata( + @EmbeddedId RecordAssetId id, + String author +) { +} diff --git a/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetRepository.java b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetRepository.java new file mode 100644 index 0000000000..dd8acc5c72 --- /dev/null +++ b/data-jdbc/src/test/java/io/micronaut/data/jdbc/h2/one2one/RecordAssetRepository.java @@ -0,0 +1,16 @@ +package io.micronaut.data.jdbc.h2.one2one; + +import io.micronaut.data.annotation.Join; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.CrudRepository; +import io.micronaut.data.repository.jpa.JpaSpecificationExecutor; + +import java.util.Optional; + +@JdbcRepository(dialect = Dialect.H2) +public interface RecordAssetRepository extends CrudRepository, JpaSpecificationExecutor { + + @Join("metadata") + Optional findById(RecordAssetId id); +} diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 7c5a0b9817..8c61cb6c9c 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -51,6 +51,7 @@ import io.micronaut.data.exceptions.MappingException; import io.micronaut.data.model.Association; import io.micronaut.data.model.DataType; +import io.micronaut.data.model.Embedded; import io.micronaut.data.model.JsonDataType; import io.micronaut.data.model.PersistentAssociationPath; import io.micronaut.data.model.PersistentEntity; @@ -2266,9 +2267,13 @@ protected void selectAllColumnsFromJoinPaths(Collection allPaths, query.append(COMMA); - boolean includeIdentity = association.isForeignKey(); - // in the case of a foreign key association the ID is not in the table, - // so we need to retrieve it + boolean includeIdentity = association.isForeignKey() + || (associatedEntity.hasIdentity() + && associatedEntity.getIdentity() instanceof Embedded + && association.getAnnotationMetadata().hasAnnotation(JoinColumns.class)); + // In the case of a foreign key association the ID is not in the owner table, + // so we need to retrieve it. Embedded IDs also need the joined aliases so + // constructor/record materialization can resolve the fetched association ID. PersistentEntityUtils.traversePersistentProperties(associatedEntity, includeIdentity, true, (propertyAssociations, prop) -> { String transformed = getDataTransformerReadValue(joinAlias, prop).orElse(null); From cac741d84573ffef64d839e6b617000cb515f7d8 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 14:27:24 +0200 Subject: [PATCH 08/20] Narrow shared-identity column reuse to explicit join columns and add duplicate embedded-column regression tests. --- .../query/builder/sql/SqlQueryBuilder.java | 41 ++++++++++--- .../builder/sql/SqlQueryBuilderUtils.java | 39 ++++++++++++ .../query/builder/sql/SqlSchemaUtils.java | 48 +++++++++++---- .../data/processor/sql/BuildInsertSpec.groovy | 61 +++++++++++++++++++ .../data/processor/sql/BuildTableSpec.groovy | 58 ++++++++++++++++++ 5 files changed, 227 insertions(+), 20 deletions(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 8c61cb6c9c..255b85e19c 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1239,13 +1239,15 @@ public JsonDataType getJsonDataType() { failOnConflictingInsertColumn(entity, unescapedColumnName, existingValueSlot.getPropertyPath(), path); } if (existingIndex == -1) { - valueSlots.add(InsertValueSlot.binding(property, path)); + boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); + valueSlots.add(InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); unescapedColumns.add(unescapedColumnName); columns.add(columnName); resultColumns.add(columnName); resultColumnTypes.add(property.getDataType()); } else { - valueSlots.set(existingIndex, InsertValueSlot.binding(property, path)); + boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); + valueSlots.set(existingIndex, InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); } }); } @@ -1278,7 +1280,7 @@ public JsonDataType getJsonDataType() { if (existingIndex != -1) { InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); - if (!isAllowedSharedIdentityColumnReuse(existingPath, path)) { + if (!isAllowedSharedIdentityColumnReuse(existingValueSlot, path)) { failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); } } @@ -1391,10 +1393,22 @@ private void failOnConflictingInsertColumn(PersistentEntity entity, String colum + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); } - private boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] existingPath, String[] identityPath) { + /** + * Allows the identity pass to replace a previously added relation join-column value. + * + *

Shared primary-key/foreign-key one-to-one mappings can surface the same physical column first through the + * relation path and later through the entity identity path. That reuse is valid only when the existing slot was + * already proven to be an explicit shared-identity join column; suffix matching alone would also match unrelated + * embedded paths such as {@code details.id}, which must remain a mapping error.

+ */ + private boolean isAllowedSharedIdentityColumnReuse(InsertValueSlot existingValueSlot, String[] identityPath) { + String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); if (existingPath == null || existingPath.length <= identityPath.length) { return false; } + if (!existingValueSlot.isSharedIdentityJoinColumn()) { + return false; + } for (int i = 1; i <= identityPath.length; i++) { if (!Objects.equals(existingPath[existingPath.length - i], identityPath[identityPath.length - i])) { return false; @@ -2461,21 +2475,30 @@ private static final class InsertValueSlot { @Nullable private final PersistentProperty property; private final String @Nullable [] propertyPath; + private final boolean sharedIdentityJoinColumn; @Nullable private final String fixedExpression; - private InsertValueSlot(@Nullable PersistentProperty property, String @Nullable [] propertyPath, @Nullable String fixedExpression) { + private InsertValueSlot(@Nullable PersistentProperty property, + String @Nullable [] propertyPath, + boolean sharedIdentityJoinColumn, + @Nullable String fixedExpression) { this.property = property; this.propertyPath = propertyPath; + this.sharedIdentityJoinColumn = sharedIdentityJoinColumn; this.fixedExpression = fixedExpression; } private static InsertValueSlot binding(PersistentProperty property, String[] propertyPath) { - return new InsertValueSlot(property, propertyPath, null); + return binding(property, propertyPath, false); + } + + private static InsertValueSlot binding(PersistentProperty property, String[] propertyPath, boolean sharedIdentityJoinColumn) { + return new InsertValueSlot(property, propertyPath, sharedIdentityJoinColumn, null); } private static InsertValueSlot literal(String fixedExpression) { - return new InsertValueSlot(null, null, fixedExpression); + return new InsertValueSlot(null, null, false, fixedExpression); } private boolean isLiteral() { @@ -2490,6 +2513,10 @@ private PersistentProperty getProperty() { return propertyPath; } + private boolean isSharedIdentityJoinColumn() { + return sharedIdentityJoinColumn; + } + private String[] getRequiredPropertyPath() { return Objects.requireNonNull(propertyPath); } diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java index fb2e23e2f4..b118628af7 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java @@ -120,6 +120,45 @@ static boolean isNotForeign(List associations) { return true; } + /** + * Detects the narrow case where a relation deliberately reuses an entity identity column. + * + *

The duplicate insert/DDL column checks use this to distinguish a valid shared primary-key/foreign-key + * one-to-one mapping from an accidental duplicate column mapping. Plain embedded paths are intentionally + * rejected because they do not have join metadata proving that the duplicate column is a shared identity column.

+ * + * @param associations The property path associations that lead to {@code property} + * @param property The associated identity property + * @param columnName The owner-side column name being written or generated + * @return {@code true} if an explicit owning relation join column maps {@code columnName} to {@code property} + */ + static boolean isExplicitSharedIdentityJoinColumn(List associations, + PersistentProperty property, + String columnName) { + Association foreignAssociation = null; + for (Association association : associations) { + if (association.getKind() != Relation.Kind.EMBEDDED) { + foreignAssociation = association; + break; + } + } + if (foreignAssociation == null || foreignAssociation.isForeignKey()) { + return false; + } + AnnotationValue joinColumns = foreignAssociation.getAnnotationMetadata().getAnnotation(JoinColumns.class); + if (joinColumns == null) { + return false; + } + for (AnnotationValue joinColumn : joinColumns.getAnnotations(AnnotationMetadata.VALUE_MEMBER)) { + String name = joinColumn.stringValue("name").orElse(null); + String referencedColumnName = joinColumn.stringValue("referencedColumnName").orElse(null); + if (columnName.equals(name) && property.getPersistedName().equals(referencedColumnName)) { + return true; + } + } + return false; + } + /** * Retrieves the joined columns from the provided annotation metadata. * diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index 1682783a41..729810e0be 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -207,13 +207,13 @@ public static List getSqlTableMappings(List List columns = new ArrayList<>(); Map identityColumnPaths = new LinkedHashMap<>(); - Map columnPaths = new LinkedHashMap<>(); + Map columnPaths = new LinkedHashMap<>(); for (PersistentProperty identity : identities) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { String columnName = namingStrategy.mappedName(associations, property); String[] path = asStringPath(associations, property); identityColumnPaths.putIfAbsent(columnName, path); - columnPaths.putIfAbsent(columnName, path); + columnPaths.putIfAbsent(columnName, new TableColumnPath(path, false)); }); } @@ -222,7 +222,7 @@ public static List getSqlTableMappings(List if (!version.isGenerated()) { String columnName = namingStrategy.mappedName(Collections.emptyList(), version); SqlColumnMapping column = getColumnDefinition(sqlColumnDefinitionProviders, version, columnName, false, true, false, dialect); - addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, new String[]{version.getName()}, column); + addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, new String[]{version.getName()}, false, column); } } @@ -230,7 +230,8 @@ public static List getSqlTableMappings(List String columnName = namingStrategy.mappedName(associations, property); SqlColumnMapping column = getColumnDefinition(sqlColumnDefinitionProviders, property, columnName, false, isRequired(associations, property), !SqlQueryBuilderUtils.isNotForeign(associations), dialect); - addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, asStringPath(associations, property), column); + boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, columnName); + addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, asStringPath(associations, property), sharedIdentityJoinColumn, column); }; for (PersistentProperty prop : entity.getPersistentProperties()) { @@ -247,36 +248,54 @@ public static List getSqlTableMappings(List return tables; } + /** + * Adds a DDL column while detecting multiple property paths mapped to the same physical column. + * + *

The only duplicate accepted here is the shared identity relation case where the table identity column is also + * used as an explicit relation join column. Other duplicate mappings would generate invalid or ambiguous DDL, so + * they fail fast instead of silently dropping one property path.

+ */ private static void addTableColumn(PersistentEntity entity, List columns, - Map columnPaths, + Map columnPaths, Map identityColumnPaths, String columnName, String[] path, + boolean sharedIdentityJoinColumn, SqlColumnMapping column) { - String[] existingPath = columnPaths.get(columnName); - if (existingPath != null) { + TableColumnPath existingColumnPath = columnPaths.get(columnName); + if (existingColumnPath != null) { + String[] existingPath = existingColumnPath.path(); String @Nullable [] identityPath = identityColumnPaths.get(columnName); - if (Arrays.equals(existingPath, path) || isAllowedSharedIdentityColumnReuse(identityPath, existingPath, path)) { + TableColumnPath newColumnPath = new TableColumnPath(path, sharedIdentityJoinColumn); + if (Arrays.equals(existingPath, path) || isAllowedSharedIdentityColumnReuse(identityPath, existingColumnPath, newColumnPath)) { return; } throw new MappingException("Conflicting table mapping for column [" + columnName + "] on entity [" + entity.getName() + "] between paths " + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); } - columnPaths.put(columnName, path); + columnPaths.put(columnName, new TableColumnPath(path, sharedIdentityJoinColumn)); columns.add(column); } + /** + * Checks whether one of two DDL paths is the identity path and the other is an explicit shared-identity relation path. + */ private static boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] identityPath, - String[] existingPath, - String[] newPath) { + TableColumnPath existingColumnPath, + TableColumnPath newColumnPath) { if (identityPath == null) { return false; } - return (Arrays.equals(existingPath, identityPath) && hasMatchingSuffix(newPath, identityPath)) - || (Arrays.equals(newPath, identityPath) && hasMatchingSuffix(existingPath, identityPath)); + String[] existingPath = existingColumnPath.path(); + String[] newPath = newColumnPath.path(); + return (Arrays.equals(existingPath, identityPath) && newColumnPath.sharedIdentityJoinColumn() && hasMatchingSuffix(newPath, identityPath)) + || (Arrays.equals(newPath, identityPath) && existingColumnPath.sharedIdentityJoinColumn() && hasMatchingSuffix(existingPath, identityPath)); } + /** + * Matches paths such as {@code metadata.id} to {@code id} without accepting the same-length identity path twice. + */ private static boolean hasMatchingSuffix(String[] longerPath, String[] shorterPath) { if (longerPath.length <= shorterPath.length) { return false; @@ -719,4 +738,7 @@ private static List getPrimaryKeyColumns(List { +} + +@MappedEntity("conflicting_embedded_insert_entity") +class ConflictingEmbeddedInsertEntity { + @Id + private Long id; + + @Relation(Relation.Kind.EMBEDDED) + private ConflictingEmbeddedInsertValue details; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + ConflictingEmbeddedInsertValue getDetails() { + return details; + } + + void setDetails(ConflictingEmbeddedInsertValue details) { + this.details = details; + } +} + +@Embeddable +class ConflictingEmbeddedInsertValue { + @MappedProperty("id") + private Long id; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } +} +""") + + then: + def ex = thrown(RuntimeException) + ex.message.contains("Conflicting insert mapping for column [id]") + } + void "shared identity sequence insert keeps contiguous postgres placeholders for r2dbc"() { given: BeanDefinition beanDefinition = buildRepository('test.SharedSequenceAssetRepository', """ diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy index be8d08c028..2c91754b88 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy @@ -141,6 +141,64 @@ class AssetMetadata { sql.count("`asset_id`") == 2 } + void "plain embedded property mapped to identity column fails ddl mapping"() { + given: + def entity = buildEntity('test.ConflictingEmbeddedEntity', ''' +import io.micronaut.data.annotation.Embeddable; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; + +@MappedEntity("conflicting_embedded_entity") +class ConflictingEmbeddedEntity { + @Id + private Long id; + + @Relation(Relation.Kind.EMBEDDED) + private ConflictingEmbeddedValue details; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + ConflictingEmbeddedValue getDetails() { + return details; + } + + void setDetails(ConflictingEmbeddedValue details) { + this.details = details; + } +} + +@Embeddable +class ConflictingEmbeddedValue { + @MappedProperty("id") + private Long id; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } +} +''') + SqlQueryBuilder builder = new SqlQueryBuilder(Dialect.H2) + + when: + builder.buildBatchCreateTableStatement(List.of(), entity) + + then: + def ex = thrown(RuntimeException) + ex.message.contains("Conflicting table mapping for column [id]") + } + @Unroll void "test build create table for JSON type for dialect #dialect"() { given: From dae48de8556eb78ee41382662862fb0d950571a5 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 14:58:16 +0200 Subject: [PATCH 09/20] Omit generated shared-identity columns from insert statements. --- .../query/builder/sql/SqlQueryBuilder.java | 5 + .../data/processor/sql/BuildInsertSpec.groovy | 88 ++++++++++ ...resOneToOneEmbeddedIdJoinColumnSpec.groovy | 153 ++++++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneEmbeddedIdJoinColumnSpec.groovy diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 255b85e19c..488aaba255 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1301,6 +1301,11 @@ public JsonDataType getJsonDataType() { if (idGeneratorType == SEQUENCE) { isSequence = true; } else if (dialect != Dialect.MYSQL || property.getDataType() != DataType.UUID) { + if (existingIndex != -1) { + // The relation pass may have added the shared identity column before we knew it was database-generated. + columns.remove(existingIndex); + valueSlots.remove(existingIndex); + } // Property skipped return; } diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index f261a94ba8..e525b4f546 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -1018,6 +1018,94 @@ class ConflictingEmbeddedInsertValue { ex.message.contains("Conflicting insert mapping for column [id]") } + void "shared identity generated insert omits generated physical id column"() { + given: + BeanDefinition beanDefinition = buildRepository('test.GeneratedSharedIdentityAssetRepository', """ +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.Relation; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import jakarta.persistence.JoinColumn; + +@JdbcRepository(dialect = Dialect.H2) +@io.micronaut.context.annotation.Executable +interface GeneratedSharedIdentityAssetRepository extends CrudRepository { +} + +@MappedEntity("generated_asset") +class GeneratedSharedIdentityAsset { + @Id + @GeneratedValue + private Long id; + + private String title; + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "id", referencedColumnName = "id") + private GeneratedSharedIdentityAssetMetadata metadata; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getTitle() { + return title; + } + + void setTitle(String title) { + this.title = title; + } + + GeneratedSharedIdentityAssetMetadata getMetadata() { + return metadata; + } + + void setMetadata(GeneratedSharedIdentityAssetMetadata metadata) { + this.metadata = metadata; + } +} + +@MappedEntity("generated_assetmetadata") +class GeneratedSharedIdentityAssetMetadata { + @Id + private Long id; + + private String author; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getAuthor() { + return author; + } + + void setAuthor(String author) { + this.author = author; + } +} +""") + + def method = beanDefinition.findPossibleMethods("save") + .toList() + .find { it.arguments.length == 1 && it.arguments[0].type.name == 'test.GeneratedSharedIdentityAsset' } + + expect: + method != null + getQuery(method) == 'INSERT INTO `generated_asset` (`title`) VALUES (?)' + getParameterPropertyPaths(method) == ["title"] as String[] + } + void "shared identity sequence insert keeps contiguous postgres placeholders for r2dbc"() { given: BeanDefinition beanDefinition = buildRepository('test.SharedSequenceAssetRepository', """ diff --git a/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneEmbeddedIdJoinColumnSpec.groovy b/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneEmbeddedIdJoinColumnSpec.groovy new file mode 100644 index 0000000000..2346372fc1 --- /dev/null +++ b/data-r2dbc/src/test/groovy/io/micronaut/data/r2dbc/postgres/one2one/PostgresOneToOneEmbeddedIdJoinColumnSpec.groovy @@ -0,0 +1,153 @@ +package io.micronaut.data.r2dbc.postgres.one2one + +import io.micronaut.context.ApplicationContext +import io.micronaut.data.annotation.Embeddable +import io.micronaut.data.annotation.EmbeddedId +import io.micronaut.data.annotation.Join +import io.micronaut.data.annotation.MappedEntity +import io.micronaut.data.annotation.MappedProperty +import io.micronaut.data.annotation.Relation +import io.micronaut.data.model.query.builder.sql.Dialect +import io.micronaut.data.r2dbc.annotation.R2dbcRepository +import io.micronaut.data.r2dbc.postgres.PostgresTestPropertyProvider +import io.micronaut.data.repository.reactive.ReactorCrudRepository +import io.micronaut.data.runtime.config.SchemaGenerate +import io.r2dbc.spi.Connection +import io.r2dbc.spi.ConnectionFactory +import io.r2dbc.spi.Result +import jakarta.persistence.JoinColumn +import reactor.core.publisher.Flux +import reactor.core.publisher.Mono +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Specification + +import java.util.UUID + +class PostgresOneToOneEmbeddedIdJoinColumnSpec extends Specification implements PostgresTestPropertyProvider { + + @AutoCleanup + @Shared + ApplicationContext context = ApplicationContext.run(properties) + + @Shared + ConnectionFactory connectionFactory = context.getBean(ConnectionFactory) + + @Shared + R2dbcAssetRepository assetRepository = context.getBean(R2dbcAssetRepository) + + @Override + SchemaGenerate schemaGenerate() { + return SchemaGenerate.NONE + } + + void setup() { + executeStatements([ + 'DROP TABLE IF EXISTS r2dbc_assetmetadata', + 'DROP TABLE IF EXISTS r2dbc_asset', + '''CREATE TABLE r2dbc_asset ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + title VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +)''', + '''CREATE TABLE r2dbc_assetmetadata ( + container_id UUID NOT NULL, + asset_id INTEGER NOT NULL, + author VARCHAR(255), + PRIMARY KEY (container_id, asset_id) +)''' + ]) + } + + void 'save owning one-to-one with composite join columns and embedded id'() { + given: + def id = new R2dbcAssetId(containerId: UUID.randomUUID(), assetId: 1) + + when: + assetRepository.save(new R2dbcAsset(id: id, title: 'title')).block() + def saved = assetRepository.findById(id).block() + + then: + saved != null + saved.id.containerId == id.containerId + saved.id.assetId == id.assetId + saved.title == 'title' + } + + void 'fetch join owning one-to-one with composite join columns and embedded id'() { + given: + def id = new R2dbcAssetId(containerId: UUID.fromString('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8'), assetId: 1) + executeStatements([ + "INSERT INTO r2dbc_assetmetadata (container_id, asset_id, author) VALUES ('${id.containerId}', ${id.assetId}, 'chris')", + "INSERT INTO r2dbc_asset (container_id, asset_id, title) VALUES ('${id.containerId}', ${id.assetId}, 'Llama Llama')" + ]) + + when: + def asset = assetRepository.findById(id).block() + + then: + asset != null + asset.metadata != null + asset.metadata.author == 'chris' + asset.metadata.id.containerId == id.containerId + asset.metadata.id.assetId == id.assetId + } + + private void executeStatements(List statements) { + Mono.usingWhen( + Mono.from(connectionFactory.create()), + { Connection connection -> + Flux.fromIterable(statements) + .concatMap { String sql -> + Flux.from(connection.createStatement(sql).execute()) + .flatMap(Result::getRowsUpdated) + .then() + } + .then() + }, + Connection::close + ).block() + } +} + +@R2dbcRepository(dialect = Dialect.POSTGRES) +interface R2dbcAssetRepository extends ReactorCrudRepository { + + @Join(value = 'metadata', type = Join.Type.LEFT_FETCH) + @Override + Mono findById(R2dbcAssetId id) +} + +@Embeddable +class R2dbcAssetId { + + @MappedProperty("container_id") + UUID containerId + + @MappedProperty("asset_id") + Integer assetId +} + +@MappedEntity("r2dbc_asset") +class R2dbcAsset { + + @EmbeddedId + R2dbcAssetId id + + String title + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "container_id", referencedColumnName = "container_id") + @JoinColumn(name = "asset_id", referencedColumnName = "asset_id") + R2dbcAssetMetadata metadata +} + +@MappedEntity("r2dbc_assetmetadata") +class R2dbcAssetMetadata { + + @EmbeddedId + R2dbcAssetId id + + String author +} From 1ed0d74db98944968bb16f81359d113ac8eb7062 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 15:12:00 +0200 Subject: [PATCH 10/20] Add H2 regression coverage for shared-id one-to-one update --- .../OneToOneEmbeddedIdJoinColumnSpec.groovy | 16 +++ .../sql/AbstractSqlLikeQueryBuilder.java | 6 +- .../data/processor/sql/BuildUpdateSpec.groovy | 110 ++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy index 028a8fb244..0cb6333e7d 100644 --- a/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy +++ b/data-jdbc/src/test/groovy/io/micronaut/data/jdbc/h2/one2one/OneToOneEmbeddedIdJoinColumnSpec.groovy @@ -69,6 +69,22 @@ CREATE TABLE assetmetadata ( saved.title == 'title' } + void 'update owning one-to-one does not write shared identity from relation path'() { + given: + def id = new AssetId(containerId: UUID.randomUUID(), assetId: 1) + assetRepository.save(new Asset(id: id, title: 'title')) + + when: + assetRepository.update(new Asset(id: id, title: 'updated', metadata: null)) + def updated = assetRepository.findById(id).orElse(null) + + then: + updated != null + updated.id.containerId == id.containerId + updated.id.assetId == id.assetId + updated.title == 'updated' + } + void 'fetch join owning one-to-one with composite join columns and embedded id'() { given: def id = new AssetId(containerId: UUID.fromString('6f8d3ed4-46e3-4656-9e89-cd61ac1e4cf8'), assetId: 1) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java index d16208afad..ee76993911 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java @@ -962,11 +962,15 @@ public JsonDataType getJsonDataType() { if (generated) { return; } + String unescapedColumnName = getMappedName(namingStrategy, associations, property); + if (SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName)) { + return; + } String tableAlias = propertyPath.getTableAlias(); if (tableAlias != null) { queryString.append(tableAlias).append(DOT); } - String columnName = getMappedName(namingStrategy, associations, property); + String columnName = unescapedColumnName; if (queryState.escape) { columnName = quote(columnName); } diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy index f99beb5204..c62a1b24fe 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy @@ -304,6 +304,116 @@ interface CompanyRepository extends CrudRepository { getParameterPropertyPaths(method) == ["name", "address.street", "address.zipCode", "hqAddress.street", "hqAddress.zipCode", "id"] as String[] } + void "shared identity join columns are not updated from relation path"() { + given: + def repository = buildRepository('test.AssetRepository', """ +import io.micronaut.data.annotation.Embeddable; +import io.micronaut.data.annotation.EmbeddedId; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import jakarta.persistence.JoinColumn; +import java.util.UUID; + +@JdbcRepository(dialect = Dialect.H2) +@io.micronaut.context.annotation.Executable +interface AssetRepository extends CrudRepository { +} + +@Embeddable +class AssetId { + @MappedProperty("container_id") + private UUID containerId; + + @MappedProperty("asset_id") + private Integer assetId; + + UUID getContainerId() { + return containerId; + } + + void setContainerId(UUID containerId) { + this.containerId = containerId; + } + + Integer getAssetId() { + return assetId; + } + + void setAssetId(Integer assetId) { + this.assetId = assetId; + } +} + +@MappedEntity("asset") +class Asset { + @EmbeddedId + private AssetId id; + + private String title; + + @Relation(value = Relation.Kind.ONE_TO_ONE, cascade = Relation.Cascade.NONE) + @JoinColumn(name = "container_id", referencedColumnName = "container_id") + @JoinColumn(name = "asset_id", referencedColumnName = "asset_id") + private AssetMetadata metadata; + + AssetId getId() { + return id; + } + + void setId(AssetId id) { + this.id = id; + } + + String getTitle() { + return title; + } + + void setTitle(String title) { + this.title = title; + } + + AssetMetadata getMetadata() { + return metadata; + } + + void setMetadata(AssetMetadata metadata) { + this.metadata = metadata; + } +} + +@MappedEntity("assetmetadata") +class AssetMetadata { + @EmbeddedId + private AssetId id; + + private String author; + + AssetId getId() { + return id; + } + + void setId(AssetId id) { + this.id = id; + } + + String getAuthor() { + return author; + } + + void setAuthor(String author) { + this.author = author; + } +} +""") + def method = repository.findPossibleMethods("update").findFirst().get() + + expect: + getQuery(method) == 'UPDATE `asset` SET `title`=? WHERE (`container_id` = ? AND `asset_id` = ?)' + getParameterPropertyPaths(method) == ["title", "id.containerId", "id.assetId"] as String[] + } void "test build update by ID"() { given: From 96d3c4c4bf8b2910b63b0e3c85142103c9405e0a Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 15:20:34 +0200 Subject: [PATCH 11/20] Narrow shared-identity update skip to identity columns --- .../sql/AbstractSqlLikeQueryBuilder.java | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java index ee76993911..930733021f 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java @@ -963,7 +963,7 @@ public JsonDataType getJsonDataType() { return; } String unescapedColumnName = getMappedName(namingStrategy, associations, property); - if (SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName)) { + if (isSharedIdentityUpdateColumn(queryState.getEntity(), namingStrategy, associations, property, unescapedColumnName)) { return; } String tableAlias = propertyPath.getTableAlias(); @@ -1023,6 +1023,47 @@ protected void appendUpdateSetParameter(StringBuilder sb, @Nullable String alias } } + /** + * Checks whether an update column comes from a relation path that reuses the entity identity column. + * + *

Normal owning relations also point at an associated identity column, but they must remain updateable + * foreign-key columns. The update skip is only safe for shared primary-key relations where the owner-side + * join column is one of the root entity identity columns and should therefore be bound from the identity + * predicate, not from the relation path.

+ */ + private boolean isSharedIdentityUpdateColumn(PersistentEntity entity, + NamingStrategy namingStrategy, + List associations, + PersistentProperty property, + String columnName) { + if (!SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, columnName)) { + return false; + } + for (PersistentProperty identity : entity.getIdentityProperties()) { + if (isIdentityColumn(namingStrategy, identity, columnName)) { + return true; + } + } + return false; + } + + /** + * Resolves the root entity identity property tree to physical column names. + * + *

This is needed for embedded ids where a single identity property expands to multiple columns. Matching + * against the resolved identity columns prevents regular foreign keys from being mistaken for shared identity + * columns during update generation.

+ */ + private boolean isIdentityColumn(NamingStrategy namingStrategy, PersistentProperty identity, String columnName) { + boolean[] match = {false}; + PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { + if (columnName.equals(getMappedName(namingStrategy, associations, property))) { + match[0] = true; + } + }); + return match[0]; + } + /** * Appends custom query part. * From 6f2bbabfb5219e2f23e7194d8c4367985d383655 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 15:37:19 +0200 Subject: [PATCH 12/20] Consolidate shared-identity SQL mapping checks --- .../sql/AbstractSqlLikeQueryBuilder.java | 43 +--------- .../query/builder/sql/SqlQueryBuilder.java | 42 +--------- .../builder/sql/SqlQueryBuilderUtils.java | 78 +++++++++++++++++++ .../query/builder/sql/SqlSchemaUtils.java | 37 ++------- 4 files changed, 88 insertions(+), 112 deletions(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java index 930733021f..103ff7e58d 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java @@ -963,7 +963,7 @@ public JsonDataType getJsonDataType() { return; } String unescapedColumnName = getMappedName(namingStrategy, associations, property); - if (isSharedIdentityUpdateColumn(queryState.getEntity(), namingStrategy, associations, property, unescapedColumnName)) { + if (SqlQueryBuilderUtils.isSharedIdentityColumn(queryState.getEntity(), namingStrategy, associations, property, unescapedColumnName)) { return; } String tableAlias = propertyPath.getTableAlias(); @@ -1023,47 +1023,6 @@ protected void appendUpdateSetParameter(StringBuilder sb, @Nullable String alias } } - /** - * Checks whether an update column comes from a relation path that reuses the entity identity column. - * - *

Normal owning relations also point at an associated identity column, but they must remain updateable - * foreign-key columns. The update skip is only safe for shared primary-key relations where the owner-side - * join column is one of the root entity identity columns and should therefore be bound from the identity - * predicate, not from the relation path.

- */ - private boolean isSharedIdentityUpdateColumn(PersistentEntity entity, - NamingStrategy namingStrategy, - List associations, - PersistentProperty property, - String columnName) { - if (!SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, columnName)) { - return false; - } - for (PersistentProperty identity : entity.getIdentityProperties()) { - if (isIdentityColumn(namingStrategy, identity, columnName)) { - return true; - } - } - return false; - } - - /** - * Resolves the root entity identity property tree to physical column names. - * - *

This is needed for embedded ids where a single identity property expands to multiple columns. Matching - * against the resolved identity columns prevents regular foreign keys from being mistaken for shared identity - * columns during update generation.

- */ - private boolean isIdentityColumn(NamingStrategy namingStrategy, PersistentProperty identity, String columnName) { - boolean[] match = {false}; - PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { - if (columnName.equals(getMappedName(namingStrategy, associations, property))) { - match[0] = true; - } - }); - return match[0]; - } - /** * Appends custom query part. * diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 488aaba255..b5bee625c7 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1232,7 +1232,7 @@ public JsonDataType getJsonDataType() { return; } - String[] path = asStringPath(associations, property); + String[] path = SqlQueryBuilderUtils.asPath(associations, property); int existingIndex = columns.indexOf(columnName); if (existingIndex != -1) { InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); @@ -1275,12 +1275,12 @@ public JsonDataType getJsonDataType() { if (escape) { columnName = quote(columnName); } - String[] path = asStringPath(associations, property); + String[] path = SqlQueryBuilderUtils.asPath(associations, property); int existingIndex = columns.indexOf(columnName); if (existingIndex != -1) { InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); - if (!isAllowedSharedIdentityColumnReuse(existingValueSlot, path)) { + if (!SqlQueryBuilderUtils.isAllowedSharedIdentityColumnReuse(path, existingPath, existingValueSlot.isSharedIdentityJoinColumn())) { failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); } } @@ -1398,42 +1398,6 @@ private void failOnConflictingInsertColumn(PersistentEntity entity, String colum + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); } - /** - * Allows the identity pass to replace a previously added relation join-column value. - * - *

Shared primary-key/foreign-key one-to-one mappings can surface the same physical column first through the - * relation path and later through the entity identity path. That reuse is valid only when the existing slot was - * already proven to be an explicit shared-identity join column; suffix matching alone would also match unrelated - * embedded paths such as {@code details.id}, which must remain a mapping error.

- */ - private boolean isAllowedSharedIdentityColumnReuse(InsertValueSlot existingValueSlot, String[] identityPath) { - String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); - if (existingPath == null || existingPath.length <= identityPath.length) { - return false; - } - if (!existingValueSlot.isSharedIdentityJoinColumn()) { - return false; - } - for (int i = 1; i <= identityPath.length; i++) { - if (!Objects.equals(existingPath[existingPath.length - i], identityPath[identityPath.length - i])) { - return false; - } - } - return true; - } - - private String[] asStringPath(List associations, PersistentProperty property) { - if (associations.isEmpty()) { - return new String[]{property.getName()}; - } - List path = new ArrayList<>(associations.size() + 1); - for (Association association : associations) { - path.add(association.getName()); - } - path.add(property.getName()); - return path.toArray(new String[0]); - } - private String getSequenceStatement(String unescapedSchemaName, String unescapedTableName, PersistentProperty property) { final String sequenceName = resolveSequenceName(property, unescapedTableName); return switch (dialect) { diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java index b118628af7..ae42d19d57 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilderUtils.java @@ -32,12 +32,14 @@ import io.micronaut.data.model.PersistentEntityUtils; import io.micronaut.data.model.PersistentProperty; import io.micronaut.data.model.naming.NamingStrategy; +import org.jspecify.annotations.Nullable; import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.OptionalInt; import java.util.function.UnaryOperator; import java.util.stream.Stream; @@ -159,6 +161,82 @@ static boolean isExplicitSharedIdentityJoinColumn(List associations return false; } + /** + * Detects the shared-identity update/DDL/insert case where an explicit join column also maps to + * one of the root entity identity columns. + * + * @param entity The root entity being written + * @param namingStrategy The naming strategy for the entity + * @param associations The property path associations that lead to {@code property} + * @param property The associated identity property + * @param columnName The owner-side physical column name + * @return {@code true} if the relation path maps to a root identity column + */ + static boolean isSharedIdentityColumn(PersistentEntity entity, + NamingStrategy namingStrategy, + List associations, + PersistentProperty property, + String columnName) { + return isExplicitSharedIdentityJoinColumn(associations, property, columnName) + && isIdentityColumn(entity, namingStrategy, columnName); + } + + /** + * Checks whether the provided physical column belongs to the root entity identity. + * + *

This resolves embedded identities to their concrete columns so callers can distinguish true shared + * identity columns from regular foreign-key columns that also reference an associated identity property.

+ */ + static boolean isIdentityColumn(PersistentEntity entity, NamingStrategy namingStrategy, String columnName) { + for (PersistentProperty identity : entity.getIdentityProperties()) { + boolean[] match = {false}; + PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { + if (columnName.equals(namingStrategy.mappedName(associations, property))) { + match[0] = true; + } + }); + if (match[0]) { + return true; + } + } + return false; + } + + /** + * Validates reuse of a relation path that intentionally shares an identity column. + * + *

The candidate path must already be proven to come from an explicit shared-identity join column, and it + * must end with the root identity path. This rejects unrelated duplicate mappings such as {@code details.id}.

+ */ + static boolean isAllowedSharedIdentityColumnReuse(String[] identityPath, + String @Nullable [] candidatePath, + boolean candidateSharedIdentityJoinColumn) { + if (!candidateSharedIdentityJoinColumn || candidatePath == null || candidatePath.length <= identityPath.length) { + return false; + } + for (int i = 1; i <= identityPath.length; i++) { + if (!Objects.equals(candidatePath[candidatePath.length - i], identityPath[identityPath.length - i])) { + return false; + } + } + return true; + } + + /** + * Converts an association/property traversal to the dot-path used by query parameter bindings and conflict checks. + */ + static String[] asPath(List associations, PersistentProperty property) { + if (associations.isEmpty()) { + return new String[]{property.getName()}; + } + List path = new ArrayList<>(associations.size() + 1); + for (Association association : associations) { + path.add(association.getName()); + } + path.add(property.getName()); + return path.toArray(new String[0]); + } + /** * Retrieves the joined columns from the provided annotation metadata. * diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index 729810e0be..a95a5c3e59 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -211,7 +211,7 @@ public static List getSqlTableMappings(List for (PersistentProperty identity : identities) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { String columnName = namingStrategy.mappedName(associations, property); - String[] path = asStringPath(associations, property); + String[] path = SqlQueryBuilderUtils.asPath(associations, property); identityColumnPaths.putIfAbsent(columnName, path); columnPaths.putIfAbsent(columnName, new TableColumnPath(path, false)); }); @@ -231,7 +231,7 @@ public static List getSqlTableMappings(List SqlColumnMapping column = getColumnDefinition(sqlColumnDefinitionProviders, property, columnName, false, isRequired(associations, property), !SqlQueryBuilderUtils.isNotForeign(associations), dialect); boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, columnName); - addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, asStringPath(associations, property), sharedIdentityJoinColumn, column); + addTableColumn(entity, columns, columnPaths, identityColumnPaths, columnName, SqlQueryBuilderUtils.asPath(associations, property), sharedIdentityJoinColumn, column); }; for (PersistentProperty prop : entity.getPersistentProperties()) { @@ -289,35 +289,10 @@ private static boolean isAllowedSharedIdentityColumnReuse(String @Nullable [] id } String[] existingPath = existingColumnPath.path(); String[] newPath = newColumnPath.path(); - return (Arrays.equals(existingPath, identityPath) && newColumnPath.sharedIdentityJoinColumn() && hasMatchingSuffix(newPath, identityPath)) - || (Arrays.equals(newPath, identityPath) && existingColumnPath.sharedIdentityJoinColumn() && hasMatchingSuffix(existingPath, identityPath)); - } - - /** - * Matches paths such as {@code metadata.id} to {@code id} without accepting the same-length identity path twice. - */ - private static boolean hasMatchingSuffix(String[] longerPath, String[] shorterPath) { - if (longerPath.length <= shorterPath.length) { - return false; - } - for (int i = 1; i <= shorterPath.length; i++) { - if (!longerPath[longerPath.length - i].equals(shorterPath[shorterPath.length - i])) { - return false; - } - } - return true; - } - - private static String[] asStringPath(List associations, PersistentProperty property) { - if (associations.isEmpty()) { - return new String[]{property.getName()}; - } - List path = new ArrayList<>(associations.size() + 1); - for (Association association : associations) { - path.add(association.getName()); - } - path.add(property.getName()); - return path.toArray(new String[0]); + return (Arrays.equals(existingPath, identityPath) + && SqlQueryBuilderUtils.isAllowedSharedIdentityColumnReuse(identityPath, newPath, newColumnPath.sharedIdentityJoinColumn())) + || (Arrays.equals(newPath, identityPath) + && SqlQueryBuilderUtils.isAllowedSharedIdentityColumnReuse(identityPath, existingPath, existingColumnPath.sharedIdentityJoinColumn())); } /** From 549bf269c5025f24f277c351f201c19542b68ca7 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 15:50:13 +0200 Subject: [PATCH 13/20] Add guard tests for explicit non-shared join column naming --- .../data/processor/sql/BuildInsertSpec.groovy | 104 ++++++++++++++++++ .../data/processor/sql/BuildQuerySpec.groovy | 67 +++++++++++ 2 files changed, 171 insertions(+) diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index e525b4f546..101410cd8f 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -378,6 +378,110 @@ interface MyInterface extends CrudRepository { getParameterPropertyPaths(method) == ["key", "carbohydrates", "portionGrams", "updatedOn", "meal.mid", "alternativeMeal.mid", "longName", "fresh", "fid"] as String[] } + void "explicit non-shared join column name is used for insert and update"() { + given: + def repository = buildRepository('test.ArticleRepository', """ +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import jakarta.persistence.JoinColumn; + +@JdbcRepository(dialect = Dialect.H2) +@io.micronaut.context.annotation.Executable +interface ArticleRepository extends CrudRepository { +} + +@MappedEntity("article") +class Article { + @Id + @GeneratedValue + private Long id; + + private String title; + + @Relation(Relation.Kind.MANY_TO_ONE) + @JoinColumn(name = "writer_key", referencedColumnName = "writer_code") + private Writer author; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getTitle() { + return title; + } + + void setTitle(String title) { + this.title = title; + } + + Writer getAuthor() { + return author; + } + + void setAuthor(Writer author) { + this.author = author; + } +} + +@MappedEntity("writer") +class Writer { + @Id + @GeneratedValue + private Long id; + + @MappedProperty("writer_code") + private Long code; + + private String name; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + Long getCode() { + return code; + } + + void setCode(Long code) { + this.code = code; + } + + String getName() { + return name; + } + + void setName(String name) { + this.name = name; + } +} +""") + + def saveMethod = repository.findPossibleMethods("save") + .toList() + .find { it.arguments.length == 1 && it.arguments[0].type.name == 'test.Article' } + def updateMethod = repository.findPossibleMethods("update").findFirst().get() + + expect: + saveMethod != null + getQuery(saveMethod) == 'INSERT INTO `article` (`title`,`writer_key`) VALUES (?,?)' + getParameterPropertyPaths(saveMethod) == ["title", "author.code"] as String[] + getQuery(updateMethod) == 'UPDATE `article` SET `title`=?,`writer_key`=? WHERE (`id` = ?)' + getParameterPropertyPaths(updateMethod) == ["title", "author.code", "id"] as String[] + } + void "test build custom SQL insert"() { given: BeanDefinition beanDefinition = buildBeanDefinition('test.MyInterface' + BeanDefinitionVisitor.PROXY_SUFFIX, """ diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildQuerySpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildQuerySpec.groovy index a91b7fbff9..9a61ae205c 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildQuerySpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildQuerySpec.groovy @@ -1575,6 +1575,73 @@ class CustomBook { getResultDataType(findAllMethod) == DataType.ENTITY } + void "test many-to-one with explicit join column name different from derived name"() { + given: + def repository = buildRepository('test.ArticleRepository', """ +import io.micronaut.data.annotation.GeneratedValue; +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.Join; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Relation; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.GenericRepository; +import jakarta.persistence.JoinColumn; + +@JdbcRepository(dialect = Dialect.H2) +@Join("author") +interface ArticleRepository extends GenericRepository { + List
findAll(); +} + +@MappedEntity("writer") +class Writer { + @GeneratedValue + @Id + private Long id; + + @MappedProperty("writer_code") + private Long code; + + private String name; + + Long getId() { return id; } + void setId(Long id) { this.id = id; } + Long getCode() { return code; } + void setCode(Long code) { this.code = code; } + String getName() { return name; } + void setName(String name) { this.name = name; } +} + +@MappedEntity("article") +class Article { + @GeneratedValue + @Id + private Long id; + + private String title; + + @Relation(Relation.Kind.MANY_TO_ONE) + @JoinColumn(name = "writer_key", referencedColumnName = "writer_code") + private Writer author; + + Long getId() { return id; } + void setId(Long id) { this.id = id; } + String getTitle() { return title; } + void setTitle(String title) { this.title = title; } + Writer getAuthor() { return author; } + void setAuthor(Writer author) { this.author = author; } +} +""") + + def findAllMethod = repository.getRequiredMethod("findAll") + + expect: + getQuery(findAllMethod) == 'SELECT article_.`id`,article_.`title`,article_.`writer_key`,article_author_.`writer_code` AS author_writer_code,article_author_.`name` AS author_name FROM `article` article_ INNER JOIN `writer` article_author_ ON article_.`writer_key`=article_author_.`writer_code`' + getResultDataType(findAllMethod) == DataType.ENTITY + } + void "test DTO with association and join"() { given: def repository = buildRepository('test.AuthorRepository', """ From 8a1470a4a9ca61557f89cf95cfa2e6e7f9d0433c Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 16:06:25 +0200 Subject: [PATCH 14/20] Remove branch build trigger --- .github/workflows/graalvm-latest.yml | 1 - .github/workflows/gradle.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/graalvm-latest.yml b/.github/workflows/graalvm-latest.yml index f377b4b4a2..e35dfe3b38 100644 --- a/.github/workflows/graalvm-latest.yml +++ b/.github/workflows/graalvm-latest.yml @@ -9,7 +9,6 @@ on: branches: - master - '[0-9]+.[0-9]+.x' - - issue-3415 pull_request: branches: - master diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 83adb838cf..63af75b3a3 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -9,7 +9,6 @@ on: branches: - master - '[0-9]+.[0-9]+.x' - - issue-3415 pull_request: branches: - master From 5aa0ac1d23e5e940667c6f11e3226c683eb136a2 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 16:06:38 +0200 Subject: [PATCH 15/20] Add some comments in the code --- .../builder/sql/AbstractSqlLikeQueryBuilder.java | 4 ++++ .../model/query/builder/sql/SqlQueryBuilder.java | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java index 103ff7e58d..91fb8eeb44 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/AbstractSqlLikeQueryBuilder.java @@ -963,6 +963,10 @@ public JsonDataType getJsonDataType() { return; } String unescapedColumnName = getMappedName(namingStrategy, associations, property); + // Shared primary-key one-to-one mappings can traverse the relation id path here + // (for example metadata.id.containerId), but those columns belong to the owner + // identity and must remain driven by the WHERE predicate instead of being written + // through the relation path in SET. if (SqlQueryBuilderUtils.isSharedIdentityColumn(queryState.getEntity(), namingStrategy, associations, property, unescapedColumnName)) { return; } diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index b5bee625c7..c0a5916d11 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1240,6 +1240,10 @@ public JsonDataType getJsonDataType() { } if (existingIndex == -1) { boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); + // Relation properties are visited before the entity identity, so an explicit shared + // PK/FK one-to-one can legitimately claim the physical column first. We record that + // fact on the slot so the identity pass below can decide whether the duplicate is + // valid shared-identity reuse or a real conflicting mapping. valueSlots.add(InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); unescapedColumns.add(unescapedColumnName); columns.add(columnName); @@ -1280,6 +1284,9 @@ public JsonDataType getJsonDataType() { if (existingIndex != -1) { InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); + // At this point the real entity identity is being processed. Reusing an existing + // column slot is allowed only when the earlier relation path was already proven to + // be an explicit shared-identity join column to this same identity path. if (!SqlQueryBuilderUtils.isAllowedSharedIdentityColumnReuse(path, existingPath, existingValueSlot.isSharedIdentityJoinColumn())) { failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); } @@ -1302,7 +1309,9 @@ public JsonDataType getJsonDataType() { isSequence = true; } else if (dialect != Dialect.MYSQL || property.getDataType() != DataType.UUID) { if (existingIndex != -1) { - // The relation pass may have added the shared identity column before we knew it was database-generated. + // The earlier relation slot was only a placeholder for a shared identity + // column. If the real identity is database-generated, that physical column + // must disappear from the INSERT instead of being bound from the relation. columns.remove(existingIndex); valueSlots.remove(existingIndex); } @@ -1317,6 +1326,8 @@ public JsonDataType getJsonDataType() { if (existingIndex == -1) { valueSlots.add(InsertValueSlot.literal(sequenceStatement)); } else { + // Shared sequence identities keep the relation-discovered column but replace its + // bound value with the sequence expression so placeholder numbering stays contiguous. valueSlots.set(existingIndex, InsertValueSlot.literal(sequenceStatement)); } } else { From cbfe607a9a0eae807c78ed19b4e9b265bbf06533 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 20 May 2026 16:52:24 +0200 Subject: [PATCH 16/20] Simplify test --- .../data/processor/sql/BuildInsertSpec.groovy | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index 101410cd8f..cb8ef59a16 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -1210,19 +1210,19 @@ class GeneratedSharedIdentityAssetMetadata { getParameterPropertyPaths(method) == ["title"] as String[] } - void "shared identity sequence insert keeps contiguous postgres placeholders for r2dbc"() { + // Simulates r2dbc positional parameters + void "shared identity sequence insert keeps contiguous postgres placeholders"() { given: BeanDefinition beanDefinition = buildRepository('test.SharedSequenceAssetRepository', """ -import io.micronaut.context.annotation.AliasFor; import io.micronaut.data.annotation.Repository; import io.micronaut.data.annotation.RepositoryConfiguration; +import io.micronaut.data.jdbc.annotation.JdbcRepository; import io.micronaut.data.model.query.builder.sql.Dialect; import io.micronaut.data.model.query.builder.sql.SqlQueryBuilder; import io.micronaut.data.model.query.builder.sql.SqlQueryConfiguration; import jakarta.persistence.JoinColumn; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; +@JdbcRepository(dialect = Dialect.POSTGRES) @RepositoryConfiguration(queryBuilder = SqlQueryBuilder.class, implicitQueries = false, namedParameters = false) @SqlQueryConfiguration( @SqlQueryConfiguration.DialectConfiguration( @@ -1230,14 +1230,6 @@ import java.lang.annotation.RetentionPolicy; positionalParameterFormat = "\$%s" ) ) -@Retention(RetentionPolicy.RUNTIME) -@Repository -@interface TestPostgresRepository { - @AliasFor(annotation = Repository.class, member = "dialect") - Dialect dialect() default Dialect.ANSI; -} - -@TestPostgresRepository(dialect = Dialect.POSTGRES) @io.micronaut.context.annotation.Executable interface SharedSequenceAssetRepository extends CrudRepository { } From 8f3bfff9549e35cde7ed9cba6b2d7ab07c8e21c1 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Wed, 27 May 2026 11:00:04 +0200 Subject: [PATCH 17/20] Address Sonar issue --- .../query/builder/sql/SqlSchemaUtils.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index a95a5c3e59..50b23de81e 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -714,6 +714,26 @@ private static List getPrimaryKeyColumns(List Date: Mon, 15 Jun 2026 15:17:16 +0200 Subject: [PATCH 19/20] Minor improvements. --- .../query/builder/sql/SqlQueryBuilder.java | 102 +++++++++--------- .../query/builder/sql/SqlSchemaUtils.java | 6 +- .../data/processor/sql/BuildInsertSpec.groovy | 12 ++- .../data/processor/sql/BuildUpdateSpec.groovy | 4 +- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index b086bd952e..6dedb35d5c 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -83,6 +83,7 @@ import java.util.Collections; import java.util.EnumMap; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -1214,8 +1215,7 @@ public JsonDataType getJsonDataType() { NamingStrategy namingStrategy = getNamingStrategy(entity); Collection persistentProperties = entity.getPersistentProperties(); - List columns = new ArrayList<>(); - List valueSlots = new ArrayList<>(); + Map valueSlotsByColumn = new LinkedHashMap<>(); for (PersistentProperty prop : persistentProperties) { PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), prop, (associations, property) -> { @@ -1233,39 +1233,30 @@ public JsonDataType getJsonDataType() { } String[] path = SqlQueryBuilderUtils.asPath(associations, property); - int existingIndex = columns.indexOf(columnName); - if (existingIndex != -1) { - InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); + InsertValueSlot existingValueSlot = valueSlotsByColumn.get(unescapedColumnName); + if (existingValueSlot != null) { failOnConflictingInsertColumn(entity, unescapedColumnName, existingValueSlot.getPropertyPath(), path); } - if (existingIndex == -1) { - boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); - // Relation properties are visited before the entity identity, so an explicit shared - // PK/FK one-to-one can legitimately claim the physical column first. We record that - // fact on the slot so the identity pass below can decide whether the duplicate is - // valid shared-identity reuse or a real conflicting mapping. - valueSlots.add(InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); - unescapedColumns.add(unescapedColumnName); - columns.add(columnName); - resultColumns.add(columnName); - resultColumnTypes.add(property.getDataType()); - } else { - boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); - valueSlots.set(existingIndex, InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); - } + boolean sharedIdentityJoinColumn = SqlQueryBuilderUtils.isExplicitSharedIdentityJoinColumn(associations, property, unescapedColumnName); + // Relation properties are visited before the entity identity, so an explicit shared + // PK/FK one-to-one can legitimately claim the physical column first. We record that + // fact on the slot so the identity pass below can decide whether the duplicate is + // valid shared-identity reuse or a real conflicting mapping. + valueSlotsByColumn.put(unescapedColumnName, InsertValueSlot.binding(property, path, sharedIdentityJoinColumn)); + unescapedColumns.add(unescapedColumnName); + resultColumns.add(columnName); + resultColumnTypes.add(property.getDataType()); }); } if (entity.hasVersion()) { PersistentProperty version = entity.getVersion(); if (!version.isGenerated()) { - valueSlots.add(InsertValueSlot.binding(version, new String[]{version.getName()})); - String columnName = getMappedName(namingStrategy, Collections.emptyList(), version); + valueSlotsByColumn.put(columnName, InsertValueSlot.binding(version, new String[]{version.getName()})); unescapedColumns.add(columnName); if (escape) { columnName = quote(columnName); } - columns.add(columnName); resultColumns.add(columnName); resultColumnTypes.add(version.getDataType()); } @@ -1280,9 +1271,8 @@ public JsonDataType getJsonDataType() { columnName = quote(columnName); } String[] path = SqlQueryBuilderUtils.asPath(associations, property); - int existingIndex = columns.indexOf(columnName); - if (existingIndex != -1) { - InsertValueSlot existingValueSlot = valueSlots.get(existingIndex); + InsertValueSlot existingValueSlot = valueSlotsByColumn.get(unescapedColumnName); + if (existingValueSlot != null) { String @Nullable [] existingPath = existingValueSlot.getPropertyPath(); // At this point the real entity identity is being processed. Reusing an existing // column slot is allowed only when the earlier relation path was already proven to @@ -1291,10 +1281,11 @@ public JsonDataType getJsonDataType() { failOnConflictingInsertColumn(entity, unescapedColumnName, existingPath, path); } } + boolean columnExists = existingValueSlot != null; boolean isSequence = false; if (SqlQueryBuilderUtils.isNotForeign(associations)) { - if (existingIndex == -1) { + if (!columnExists) { unescapedColumns.add(unescapedColumnName); resultColumns.add(columnName); resultColumnTypes.add(property.getDataType()); @@ -1308,12 +1299,11 @@ public JsonDataType getJsonDataType() { if (idGeneratorType == SEQUENCE) { isSequence = true; } else if (dialect != Dialect.MYSQL || property.getDataType() != DataType.UUID) { - if (existingIndex != -1) { + if (columnExists) { // The earlier relation slot was only a placeholder for a shared identity // column. If the real identity is database-generated, that physical column // must disappear from the INSERT instead of being bound from the relation. - columns.remove(existingIndex); - valueSlots.remove(existingIndex); + valueSlotsByColumn.remove(unescapedColumnName); } // Property skipped return; @@ -1323,31 +1313,23 @@ public JsonDataType getJsonDataType() { if (isSequence) { String sequenceStatement = getSequenceStatement(unescapedSchema, unescapedTableName, property); - if (existingIndex == -1) { - valueSlots.add(InsertValueSlot.literal(sequenceStatement)); - } else { - // Shared sequence identities keep the relation-discovered column but replace its - // bound value with the sequence expression so placeholder numbering stays contiguous. - valueSlots.set(existingIndex, InsertValueSlot.literal(sequenceStatement)); - } + // Shared sequence identities keep the relation-discovered column but replace its + // bound value with the sequence expression so placeholder numbering stays contiguous. + valueSlotsByColumn.put(unescapedColumnName, InsertValueSlot.literal(sequenceStatement)); } else { - if (existingIndex == -1) { - valueSlots.add(InsertValueSlot.binding(property, path)); - } else { - valueSlots.set(existingIndex, InsertValueSlot.binding(property, path)); - } - } - - if (existingIndex == -1) { - columns.add(columnName); + valueSlotsByColumn.put(unescapedColumnName, InsertValueSlot.binding(property, path)); } }); } - List values = new ArrayList<>(valueSlots.size()); - parameterBindings = new ArrayList<>(valueSlots.size()); + List columns = valueSlotsByColumn.keySet() + .stream() + .map(columnName -> escape ? quote(columnName) : columnName) + .toList(); + List values = new ArrayList<>(valueSlotsByColumn.size()); + parameterBindings = new ArrayList<>(valueSlotsByColumn.size()); int bindingIndex = 1; - for (InsertValueSlot valueSlot : valueSlots) { + for (InsertValueSlot valueSlot : valueSlotsByColumn.values()) { if (valueSlot.isLiteral()) { values.add(valueSlot.getFixedExpression()); continue; @@ -2469,6 +2451,28 @@ private static InsertValueSlot literal(String fixedExpression) { return new InsertValueSlot(null, null, false, fixedExpression); } + @Override + public boolean equals(Object obj) { + return obj instanceof InsertValueSlot(var otherProperty, var otherPropertyPath, var otherSharedIdentityJoinColumn, var otherFixedExpression) + && Objects.equals(property, otherProperty) + && Arrays.equals(propertyPath, otherPropertyPath) + && sharedIdentityJoinColumn == otherSharedIdentityJoinColumn + && Objects.equals(fixedExpression, otherFixedExpression); + } + + @Override + public int hashCode() { + int result = Objects.hash(property, sharedIdentityJoinColumn, fixedExpression); + result = 31 * result + Arrays.hashCode(propertyPath); + return result; + } + + @Override + public String toString() { + return "InsertValueSlot[property=" + property + ", propertyPath=" + Arrays.toString(propertyPath) + + ", sharedIdentityJoinColumn=" + sharedIdentityJoinColumn + ", fixedExpression=" + fixedExpression + ']'; + } + private boolean isLiteral() { return fixedExpression != null; } diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index 50b23de81e..527e429d04 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -719,9 +719,9 @@ private record TableColumnPath(String[] path, boolean sharedIdentityJoinColumn) @Override public boolean equals(Object obj) { - return obj instanceof TableColumnPath other - && Arrays.equals(path, other.path) - && sharedIdentityJoinColumn == other.sharedIdentityJoinColumn; + return obj instanceof TableColumnPath(String[] otherPath, boolean otherSharedIdentityJoinColumn) + && Arrays.equals(path, otherPath) + && sharedIdentityJoinColumn == otherSharedIdentityJoinColumn; } @Override diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index cb8ef59a16..b20126bb01 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -1131,11 +1131,13 @@ import io.micronaut.data.annotation.MappedEntity; import io.micronaut.data.annotation.Relation; import io.micronaut.data.jdbc.annotation.JdbcRepository; import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.GenericRepository; import jakarta.persistence.JoinColumn; @JdbcRepository(dialect = Dialect.H2) @io.micronaut.context.annotation.Executable -interface GeneratedSharedIdentityAssetRepository extends CrudRepository { +interface GeneratedSharedIdentityAssetRepository extends GenericRepository { + GeneratedSharedIdentityAsset insert(GeneratedSharedIdentityAsset entity); } @MappedEntity("generated_asset") @@ -1200,7 +1202,7 @@ class GeneratedSharedIdentityAssetMetadata { } """) - def method = beanDefinition.findPossibleMethods("save") + def method = beanDefinition.findPossibleMethods("insert") .toList() .find { it.arguments.length == 1 && it.arguments[0].type.name == 'test.GeneratedSharedIdentityAsset' } @@ -1214,6 +1216,7 @@ class GeneratedSharedIdentityAssetMetadata { void "shared identity sequence insert keeps contiguous postgres placeholders"() { given: BeanDefinition beanDefinition = buildRepository('test.SharedSequenceAssetRepository', """ +import io.micronaut.data.repository.GenericRepository; import io.micronaut.data.annotation.Repository; import io.micronaut.data.annotation.RepositoryConfiguration; import io.micronaut.data.jdbc.annotation.JdbcRepository; @@ -1231,7 +1234,8 @@ import jakarta.persistence.JoinColumn; ) ) @io.micronaut.context.annotation.Executable -interface SharedSequenceAssetRepository extends CrudRepository { +interface SharedSequenceAssetRepository extends GenericRepository { + SharedSequenceAsset insert(SharedSequenceAsset entity); } @MappedEntity("sequence_asset") @@ -1307,7 +1311,7 @@ class SharedSequenceAssetMetadata { } """) - def method = beanDefinition.findPossibleMethods("save") + def method = beanDefinition.findPossibleMethods("insert") .toList() .find { it.arguments.length == 1 && it.arguments[0].type.name == 'test.SharedSequenceAsset' } diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy index c62a1b24fe..51fe7f0ede 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildUpdateSpec.groovy @@ -314,12 +314,14 @@ import io.micronaut.data.annotation.MappedProperty; import io.micronaut.data.annotation.Relation; import io.micronaut.data.jdbc.annotation.JdbcRepository; import io.micronaut.data.model.query.builder.sql.Dialect; +import io.micronaut.data.repository.GenericRepository; import jakarta.persistence.JoinColumn; import java.util.UUID; @JdbcRepository(dialect = Dialect.H2) @io.micronaut.context.annotation.Executable -interface AssetRepository extends CrudRepository { +interface AssetRepository extends GenericRepository { + Asset update(Asset entity); } @Embeddable From 7054ee3be6c060c7e694e2dd9ec613707f7db169 Mon Sep 17 00:00:00 2001 From: radovanradic Date: Mon, 15 Jun 2026 16:07:25 +0200 Subject: [PATCH 20/20] Address review comments. --- .../query/builder/sql/SqlQueryBuilder.java | 7 ++- .../query/builder/sql/SqlSchemaUtils.java | 15 ++++- .../data/processor/sql/BuildInsertSpec.groovy | 58 +++++++++++++++++++ .../data/processor/sql/BuildTableSpec.groovy | 52 +++++++++++++++++ 4 files changed, 129 insertions(+), 3 deletions(-) diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java index 6dedb35d5c..4cda45ab1f 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlQueryBuilder.java @@ -1252,7 +1252,12 @@ public JsonDataType getJsonDataType() { PersistentProperty version = entity.getVersion(); if (!version.isGenerated()) { String columnName = getMappedName(namingStrategy, Collections.emptyList(), version); - valueSlotsByColumn.put(columnName, InsertValueSlot.binding(version, new String[]{version.getName()})); + String[] path = new String[]{version.getName()}; + InsertValueSlot existingValueSlot = valueSlotsByColumn.get(columnName); + if (existingValueSlot != null) { + failOnConflictingInsertColumn(entity, columnName, existingValueSlot.getPropertyPath(), path); + } + valueSlotsByColumn.put(columnName, InsertValueSlot.binding(version, path)); unescapedColumns.add(columnName); if (escape) { columnName = quote(columnName); diff --git a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java index 527e429d04..e6f86e3b22 100644 --- a/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java +++ b/data-model/src/main/java/io/micronaut/data/model/query/builder/sql/SqlSchemaUtils.java @@ -212,8 +212,14 @@ public static List getSqlTableMappings(List PersistentEntityUtils.traversePersistentProperties(Collections.emptyList(), identity, (associations, property) -> { String columnName = namingStrategy.mappedName(associations, property); String[] path = SqlQueryBuilderUtils.asPath(associations, property); - identityColumnPaths.putIfAbsent(columnName, path); - columnPaths.putIfAbsent(columnName, new TableColumnPath(path, false)); + String @Nullable [] existingIdentityPath = identityColumnPaths.putIfAbsent(columnName, path); + if (existingIdentityPath != null && !Arrays.equals(existingIdentityPath, path)) { + failOnConflictingIdentityColumn(entity, columnName, existingIdentityPath, path); + } + TableColumnPath existingColumnPath = columnPaths.putIfAbsent(columnName, new TableColumnPath(path, false)); + if (existingColumnPath != null && !Arrays.equals(existingColumnPath.path(), path)) { + failOnConflictingIdentityColumn(entity, columnName, existingColumnPath.path(), path); + } }); } @@ -248,6 +254,11 @@ public static List getSqlTableMappings(List return tables; } + private static void failOnConflictingIdentityColumn(PersistentEntity entity, String columnName, String[] existingPath, String[] path) { + throw new MappingException("Conflicting identity mapping for column [" + columnName + "] on entity [" + entity.getName() + "] between paths " + + Arrays.toString(existingPath) + " and " + Arrays.toString(path)); + } + /** * Adds a DDL column while detecting multiple property paths mapped to the same physical column. * diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy index b20126bb01..08f43a2f63 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildInsertSpec.groovy @@ -1061,6 +1061,64 @@ class ConflictingInsertEntity { ex.message.contains("Conflicting insert mapping for column [shared_value]") } + void "property mapped to version column fails insert mapping"() { + when: + buildRepository('test.ConflictingVersionInsertEntityRepository', """ +import io.micronaut.data.annotation.Id; +import io.micronaut.data.annotation.MappedEntity; +import io.micronaut.data.annotation.MappedProperty; +import io.micronaut.data.annotation.Version; +import io.micronaut.data.jdbc.annotation.JdbcRepository; +import io.micronaut.data.model.query.builder.sql.Dialect; + +@JdbcRepository(dialect = Dialect.H2) +@io.micronaut.context.annotation.Executable +interface ConflictingVersionInsertEntityRepository extends CrudRepository { +} + +@MappedEntity("conflicting_version_insert_entity") +class ConflictingVersionInsertEntity { + @Id + private Long id; + + @MappedProperty("lock_value") + private String name; + + @Version + @MappedProperty("lock_value") + private Long version; + + Long getId() { + return id; + } + + void setId(Long id) { + this.id = id; + } + + String getName() { + return name; + } + + void setName(String name) { + this.name = name; + } + + Long getVersion() { + return version; + } + + void setVersion(Long version) { + this.version = version; + } +} +""") + + then: + def ex = thrown(RuntimeException) + ex.message.contains("Conflicting insert mapping for column [lock_value]") + } + void "plain embedded property mapped to identity column fails insert mapping"() { when: buildRepository('test.ConflictingEmbeddedInsertEntityRepository', """ diff --git a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy index 2c91754b88..31aeb10a41 100644 --- a/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy +++ b/data-processor/src/test/groovy/io/micronaut/data/processor/sql/BuildTableSpec.groovy @@ -141,6 +141,58 @@ class AssetMetadata { sql.count("`asset_id`") == 2 } + void "duplicate physical identity columns fail ddl mapping"() { + given: + def entity = buildEntity('test.ConflictingIdentityEntity', ''' +@MappedEntity("conflicting_identity_entity") +class ConflictingIdentityEntity { + @EmbeddedId + private ConflictingIdentity id; + + ConflictingIdentity getId() { + return id; + } + + void setId(ConflictingIdentity id) { + this.id = id; + } +} + +@Embeddable +class ConflictingIdentity { + @MappedProperty("shared_id") + private Long firstId; + + @MappedProperty("shared_id") + private Long secondId; + + Long getFirstId() { + return firstId; + } + + void setFirstId(Long firstId) { + this.firstId = firstId; + } + + Long getSecondId() { + return secondId; + } + + void setSecondId(Long secondId) { + this.secondId = secondId; + } +} +''') + SqlQueryBuilder builder = new SqlQueryBuilder(Dialect.H2) + + when: + builder.buildBatchCreateTableStatement(List.of(), entity) + + then: + def ex = thrown(RuntimeException) + ex.message.contains("Conflicting identity mapping for column [shared_id]") + } + void "plain embedded property mapped to identity column fails ddl mapping"() { given: def entity = buildEntity('test.ConflictingEmbeddedEntity', '''