jdaugherty commented on code in PR #16539:
URL: https://github.com/apache/grails-core/pull/16539#discussion_r4230979723
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/hibernate/GrailsHibernatePersistentEntity.java:
##########
@@ -384,9 +385,10 @@ default boolean isLazy(HibernatePersistentProperty
property) {
default void sortOrIndexForeignKeyColumns(SimpleValue value) {
PersistentClass pc = getPersistentClass();
KeyValue identifier = pc != null ? pc.getIdentifier() : null;
- int[] originalOrder = identifier instanceof Component c ?
c.sortProperties() : null;
+ Component component = identifier instanceof Component c ? c : null;
+ int[] originalOrder = component != null ? component.sortProperties() :
null;
if (originalOrder != null) {
- value.sortColumns(originalOrder);
+ value.sortColumns(toColumnPermutation(originalOrder,
partColumnSpans(component, originalOrder)));
} else {
Review Comment:
0b0be81a04 fixes this for the to-one and join-table keys, but one order of
the same three classes still fails:
```groovy
new HibernateDatastore(config, PrbMiddle, PrbLeaf, PrbGrand)
```
```
Referential integrity constraint violation: "FKDEN36EQGOPJY1BD3T41V9BTYS:
PUBLIC.PRB_LEAF FOREIGN KEY(PRB_MIDDLE_GRAND_PARENT_ALPHA,
PRB_MIDDLE_GRAND_PARENT_ZETA, PRB_MIDDLE_NAME) REFERENCES
PUBLIC.PRB_MIDDLE(NAME, PRB_GRAND_ALPHA, PRB_GRAND_ZETA) ('a', 'z', 'm')"
```
The foreign key that reaches the database here isn't the binder's. The
inverse `PrbMiddle.leaves` key gets its own positional foreign key from
`collection.createAllKeys()` (through `GrailsSecondPass.createCollectionKeys`),
under the same name as the binder's explicit key for `PrbLeaf.middle`. Both
keys are in the metadata in every order, and only the first one registered is
created. The second fails with `Constraint "FKDEN36EQGOPJY1BD3T41V9BTYS"
already exists`, which `CompositeForeignKeyRegistrationOrderSpec` logs on every
run. In this order the collection's second pass is queued before the
`CompositeForeignKeySecondPass` for `PrbLeaf.middle`, so the positional key is
the one created. A bidirectional `hasMany` into the `PrbHub` layout fails the
same way when registered as hub, child, grand.
As a local check, I disabled the collection key's foreign key in
`BidirectionalOneToManyLinker.link`, leaving the to-one side to create it. All
six orders then save, and the duplicate-name errors go away. I only ran the
composite specs with that change.
Could you fix this order in this PR? `[PrbGrand, PrbMiddle,
PrbLeaf].permutations()` in the `where:` block would cover all six orders.
##########
grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/CompositeForeignKeyRegistrationOrderSpec.groovy:
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.grails.orm.hibernate.cfg.domainbinding.binder
+
+import org.grails.orm.hibernate.HibernateDatastore
+import org.hibernate.dialect.H2Dialect
+import spock.lang.Specification
+import spock.lang.Unroll
+
+/**
+ * An application registers its entities in class-path scan order, so an
entity is often bound before
+ * the entity its composite foreign key references. The key must still be
aligned with the referenced
+ * identifier once that identifier is bound, exactly as when the referenced
entity is registered first.
+ * The entities are the ones of {@link CompositeForeignKeyColumnTypesSpec},
which registers every
+ * referenced entity first.
+ */
+class CompositeForeignKeyRegistrationOrderSpec extends Specification {
+
+ private static final List<List<String>> LEAF_KEY = [
+ ['prb_middle_grand_parent_alpha', 'prb_grand_alpha'],
+ ['prb_middle_grand_parent_zeta', 'prb_grand_zeta'],
+ ['prb_middle_name', 'name']]
+
+ private static final List<List<String>> HUB_KEY = [
+ ['prb_hub_ace', 'ace'],
+ ['prb_hub_grand_alpha', 'prb_grand_alpha'],
+ ['prb_hub_grand_zeta', 'prb_grand_zeta'],
+ ['prb_hub_zed', 'zed']]
+
+ private static Map<String, Object> config(List<Class> classes) {
+ [
+ 'dataSource.url' :
"jdbc:h2:mem:compositeFkOrder${classes*.simpleName.join('')};LOCK_TIMEOUT=10000".toString(),
+ 'dataSource.dbCreate' : 'create-drop',
+ 'dataSource.dialect' : H2Dialect.name,
+ 'hibernate.hbm2ddl.auto': 'create',
+ ]
+ }
+
+ @Unroll
+ void "the keys of a three level composite chain registered as #order are
aligned with the keys they reference"() {
+ given:
+ HibernateDatastore datastore = new HibernateDatastore(config(classes),
classes as Class[])
+
+ when:
+ Map<String, List<List<String>>> middleKeys =
ForeignKeyPairs.of(datastore, 'PRB_MIDDLE')
+ Map<String, List<List<String>>> leafKeys =
ForeignKeyPairs.of(datastore, 'PRB_LEAF')
+
+ then:
+ middleKeys == [prb_grand: [['prb_grand_alpha', 'alpha'],
['prb_grand_zeta', 'zeta']]]
+ leafKeys == [prb_middle: LEAF_KEY]
+
+ when:
+ PrbGrand.withNewTransaction {
+ PrbGrand grand = new PrbGrand(zeta: 'z', alpha:
'a').save(failOnError: true)
+ PrbMiddle middle = new PrbMiddle(name: 'm', grandParent:
grand).save(failOnError: true)
+ new PrbLeaf(name: 'l', middle: middle).save(failOnError: true,
flush: true)
+ }
+
+ then:
+ PrbLeaf.withNewSession {
+ PrbLeaf leaf = PrbLeaf.findByName('l')
+ leaf.middle.name == 'm' && leaf.middle.grandParent.alpha == 'a' &&
leaf.middle.grandParent.zeta == 'z'
Review Comment:
This reload only follows `leaf.middle`. `grand.middles` and `middle.leaves`
load empty here, in every registration order. This predates the PR and doesn't
need nesting. On 8.0.x, a parent with `composite('zeta', 'alpha')` and `hasMany
= [kids: Kid]`, where `Kid` `belongsTo` the parent, loads no kids. Declared as
`composite('alpha', 'zeta')`, it loads them.
`BidirectionalOneToManyLinker.link` copies the to-one's columns, which are
already in key order. Then `key.sortProperties()` applies the owner
identifier's permutation a second time. With the referenced entities registered
first, `PrbMiddle.leaves` gets `prb_middle_grand_parent_zeta,
prb_middle_grand_parent_alpha, prb_middle_name` while `PrbLeaf.middle` is
`prb_middle_grand_parent_alpha, prb_middle_grand_parent_zeta, prb_middle_name`.
So the collection is loaded with the identifier values bound to the wrong
columns. I tried marking the copied key sorted instead of re-sorting it. The
collections then load in every order except the two where the collection's
second pass runs before the to-one's deferred alignment. There, the to-one
would need aligning before it is copied.
This is the same method as the foreign key in the other thread. Would you
take it in this PR, or should we open a follow-up issue?
--
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]
