jdaugherty commented on code in PR #16720:
URL: https://github.com/apache/grails-core/pull/16720#discussion_r4352212930
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/CreateKeyForProps.java:
##########
@@ -70,9 +72,15 @@ public void createKeyForProps(HibernatePersistentProperty
grailsProp, String pat
throw new MappingException(
owner.getJavaClass().getName() + " references an
unknown property " + propertyName);
}
- String otherColumnName =
-
columnNameForPropertyAndPathFetcher.getColumnNameForPropertyAndPath(otherProp,
path, null);
- keyList.add(new Column(otherColumnName));
+ List<ColumnConfig> columns =
otherProp.getHibernateMappedForm().getColumns();
+ if (columns.isEmpty()) {
Review Comment:
This only resolves the right columns if the association is already bound.
For a composite-id association without a `column` mapping, this list stays
empty until
`CompositeIdentifierToManyToOneBinder.bindCompositeIdentifierToManyToOne` adds
the generated configs (`<parent table>_code`, `<parent table>_revision`) while
it binds the association. Properties bind in declaration order. When the
constrained property is declared first, this code therefore falls back to the
single default name `parent_id`, and no column has that name:
```groovy
@Entity
class EnumUniqueDeclaredFirstChild implements
HibernateEntity<EnumUniqueDeclaredFirstChild> {
EnumUniqueCompositeState state
EnumUniqueCompositeParent parent
static mapping = {
state unique: 'parent'
}
}
```
On this head the `create table` ends with `unique (parent_id, state)`. With
`hibernate.hbm2ddl.halt_on_error`, the datastore fails to start with `Column
"PARENT_ID" not found`. With the default setting, the datastore starts without
the table, and the first save throws `Table ... not found`. If the two
declarations are swapped, the mapping works and the key is `(<parent
table>_code, <parent table>_revision, state)`. 8.0.x fails the same way on this
mapping. Enum binding did not create group keys before #16533, so for an enum
this is still part of the #16713 regression. A `String` member declared first
fails the same way too, on 8.0.x as well.
The result shouldn't depend on declaration order. I see two possible routes:
- Resolve an unbound to-one member's columns from the referenced composite
identity, using the naming that `CompositeIdentifierToManyToOneBinder` applies
(shared, not copied).
- Build the group key once every property of the entity is bound, from the
bound `Column`s of the member's `Property`. This would also keep their quoting.
Could you fix this and add the declared-first case, for both an enum and a
`String`, to `EnumUniqueGroupAssociationSpec`?
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/util/CreateKeyForProps.java:
##########
@@ -70,9 +72,15 @@ public void createKeyForProps(HibernatePersistentProperty
grailsProp, String pat
throw new MappingException(
owner.getJavaClass().getName() + " references an
unknown property " + propertyName);
}
- String otherColumnName =
-
columnNameForPropertyAndPathFetcher.getColumnNameForPropertyAndPath(otherProp,
path, null);
- keyList.add(new Column(otherColumnName));
+ List<ColumnConfig> columns =
otherProp.getHibernateMappedForm().getColumns();
+ if (columns.isEmpty()) {
+ columns = Collections.singletonList(null);
+ }
+ for (ColumnConfig column : columns) {
+ String otherColumnName =
+
columnNameForPropertyAndPathFetcher.getColumnNameForPropertyAndPath(otherProp,
path, column);
+ keyList.add(new Column(otherColumnName));
+ }
}
uniqueKeyForColumnsCreator.createUniqueKeyForColumns(table,
keyList);
Review Comment:
Non-blocking, and older than this PR. This key is built once per bound
column of the constrained property: `ColumnBinder.bindColumn` calls
`createKeyForProps` for each column. As a result, the reverse mapping, where
the multi-column association is the constrained property, still produces the
wrong keys:
```groovy
static mapping = {
parent(unique: 'state') {
column name: 'parent_code'
column name: 'parent_revision'
}
}
```
This gives two keys, `(parent_code, state)` and `(parent_revision, state)`.
A child is then rejected when its parent shares only `code` with the parent of
an existing child that has the same state. Without the `column` mapping, the
datastore fails to start with `MappingException: UniqueKey UK… already exists`.
8.0.x behaves the same. #16695 handles this shape for a composite user type by
creating one key across all of the property's columns (`createKeyForColumns`),
and a multi-column to-one could do the same. A follow-up issue is fine if you'd
rather keep this PR to #16713.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
