jdaugherty commented on code in PR #16701: URL: https://github.com/apache/grails-core/pull/16701#discussion_r4383761007
########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/secondpass/SharedJoinTableUniqueKeyBinder.java: ########## @@ -0,0 +1,286 @@ +/* + * 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.secondpass; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.hibernate.MappingException; +import org.hibernate.boot.Metadata; +import org.hibernate.boot.model.naming.Identifier; +import org.hibernate.boot.model.naming.ImplicitIndexNameSource; +import org.hibernate.boot.model.naming.ImplicitNamingStrategy; +import org.hibernate.boot.model.naming.ImplicitUniqueKeyNameSource; +import org.hibernate.boot.spi.MetadataBuildingContext; +import org.hibernate.dialect.Dialect; +import org.hibernate.mapping.Collection; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.IdentifierCollection; +import org.hibernate.mapping.IndexedCollection; +import org.hibernate.mapping.Selectable; +import org.hibernate.mapping.Table; +import org.hibernate.mapping.Value; + +/** + * Gives every collection with a join table a uniqueness constraint of its own over its key column and the element, + * so that a join table several collections of one owner share stays unique per collection. + * + * <p>Two unidirectional {@code hasMany} to the same class without a {@code joinTable} mapping share one join table, + * with a key column for each of them and one element column. Hibernate creates the table's implicit unique key when + * it creates the keys of the first collection, over that collection's key column and the element column, and creates + * none for the others, as the table has a unique key already ({@code Set.createPrimaryKey}). This binder runs after + * the keys of each collection are created and completes what is missing, in the shape the database needs + * ({@link SharedJoinTableUniqueness}). A collection whose table is its own already has its constraint and is left as + * Hibernate bound it, and so is a collection without a join table, whose key the other side owns, or whose element is + * a large object, which cannot be indexed and which Hibernate leaves without a key too. It also restricts + * each collection of a shared table to its own rows, which Hibernate's removal of all the rows of a collection by its + * key would otherwise not.</p> + * + * <p>Hibernate applies the restriction to the statements that load, count, join and delete the rows of a collection, + * but loses it in the test of emptiness ({@code is empty}, {@code is not empty}); GORM translates that test as a size + * test for a restricted many-to-many collection instead, through + * {@link org.grails.orm.hibernate.query.RestrictedCollectionSqmTranslatorFactory} (apache/grails-core#16748), except in + * an update or delete of an entity whose hierarchy spans several tables.</p> + * + * <p>A set cannot share a join table with a list or a map: the index column of a list or a map is not null, and the + * rows of the set leave it null, so saving the set fails on the first insert.</p> + * + * @since 8.0.1 + */ +public class SharedJoinTableUniqueKeyBinder { + + private final MetadataBuildingContext metadataBuildingContext; + private final SharedJoinTableUniqueness uniqueness; + + /** Resolves the uniqueness for the dialect of the metadata being built. */ + public SharedJoinTableUniqueKeyBinder(MetadataBuildingContext metadataBuildingContext) { + this(metadataBuildingContext, metadataBuildingContext.getMetadataCollector().getDatabase().getDialect()); + } + + SharedJoinTableUniqueKeyBinder(MetadataBuildingContext metadataBuildingContext, Dialect dialect) { + this.metadataBuildingContext = metadataBuildingContext; + this.uniqueness = SharedJoinTableUniqueness.forDialect(dialect); + } + + /** The uniqueness this binder applies. */ + public SharedJoinTableUniqueness getUniqueness() { + return uniqueness; + } + + /** + * Gives a set the uniqueness constraint over its key columns and its element on its join table. A list or a map + * is keyed by its index, which its table has as part of the primary key, and a bag has no key at all; their + * elements repeat by design, so they are left as Hibernate bound them. + */ + public void bind(Collection collection) { + Table table = collection.getCollectionTable(); + if (table == null || collection.isInverse() || collection.isOneToMany() || collection.getKey() == null) { + return; + } + restrictRowsToOwnCollection(collection, table); + if (!collection.isSet()) { + return; + } + List<Column> keyColumns = collection.getKey().getColumns(); + List<Column> elementColumns = columnsOf(collection); + if (keyColumns.isEmpty() || elementColumns.isEmpty() || hasLobColumn(collection.getMetadata(), elementColumns)) { + return; + } + uniqueness.apply(table, keyColumns, elementColumns, new ImplicitNaming(table)); + } + + /** + * Hibernate removes all the rows of a collection with one statement on its key, and loads them with one. On a join + * table several collections share, a key column two of them use would take the rows of the other collection + * along, so each collection of such a table is restricted to the rows that have a value in its own element + * columns. The collections that share the table are found among the bindings of the metadata; one whose element is + * not bound yet is restricted when its own pass runs, which finds the others bound. + */ + private void restrictRowsToOwnCollection(Collection collection, Table table) { + if (!hasColumnsOfOthers(collection, table)) { + return; + } + List<Collection> sharing = new ArrayList<>(); + for (Collection other : metadataBuildingContext.getMetadataCollector().getCollectionBindings()) { + if (other.getCollectionTable() == table && !other.isInverse() && !other.isOneToMany() && other.getKey() != null) { + sharing.add(other); + } + } + if (sharing.size() < 2) { + return; + } + Dialect dialect = metadataBuildingContext.getMetadataCollector().getDatabase().getDialect(); + for (Collection member : sharing) { + List<String> present = new ArrayList<>(); + for (Column column : columnsOf(member)) { + present.add(column.getQuotedName(dialect) + " is not null"); + } + if (present.isEmpty()) { + continue; + } + String restriction = String.join(" and ", present); + String where = member.getWhere(); + if (where == null || where.isBlank()) { + member.setWhere(restriction); + } else if (!where.contains(restriction)) { + member.setWhere("(" + where + ") and " + restriction); + } + } + } + + /** + * Whether the table holds a column that the collection does not use: its key, element, index and identifier + * columns are its own, and a column besides those belongs to another collection that shares the table. A table + * only this collection uses is not searched for the others, which keeps the binding of an application with many + * collections linear. The collection of a shared table that is bound first finds none, and the one bound after + * it restricts both. + */ + private static boolean hasColumnsOfOthers(Collection collection, Table table) { Review Comment: Non-blocking. `hasColumnsOfOthers` is also true for the default shape, `hasMany = [allow: Member, deny: Member]` with no mapping. There each association has its own key column and both share the element column, so both collections get the same `member_id is not null`. The binder spec pins this for `SjtOwner` (`roles.where == 'sjt_role_id is not null'`, lines 104-105). The predicate separates nothing in that shape, because each association's rows are already selected and deleted by its own key column. Captured with a statement inspector on H2 at e635103fa2: ``` load: ... where a1_0.p16701_group_allow_id=? and (a1_0.p16701_member_id is not null) clear: delete from p16701_group_p16701_member where p16701_group_allow_id=? and ((p16701_member_id is not null)) is empty: ... where (select count(*) from p16701_group_p16701_member d1_0 where (d1_0.p16701_member_id is not null) and pg1_0.id=d1_0.p16701_group_deny_id)=0 ``` An owner with a table of its own keeps `not exists(select 1 ...)` for `is empty`. Because the default shape now `hasWhereRestrictions()`, `RestrictedCollectionSqmTranslatorFactory` also turns its emptiness test into a count. The restriction is needed only when the sharing collections have a key column in common, which is the `joinTable` shape of `SjtbdAccount` in `SharedJoinTableBulkDeleteSpec`. Restricting only that case would leave the SQL of the most common shared shape as it was, and would keep the translator override to the one case that needs it. ########## grails-doc/src/en/guide/upgrading/upgrading80x.adoc: ########## @@ -1460,7 +1460,7 @@ A type that spans several columns no longer implements `UserType` with several ` Under Hibernate 7, an owner with two or more unidirectional `hasMany` associations wrote no rows to their join tables: an element added to any of them was lost at flush (issue https://github.com/apache/grails-core/issues/16665[#16665]). This is fixed when each association has a join table of its own, which is the default for associations to different classes (`item_roles` and `item_tags` for `static hasMany = [roles: Role, tags: Tag]`) and also the case when two associations to the same class map different tables with `joinTable`. Each such join table is written, as under Hibernate 5. -Two unidirectional associations to the same class share one join table by default, named after the owner and the element class, with a key column per association (`item_role` with `item_roles_id` and `item_deny_roles_id` for `static hasMany = [roles: Role, denyRoles: Role]`). Such a table is read, but new elements are not written, and neither are those of associations that `joinTable` maps onto one table. That is issue https://github.com/apache/grails-core/issues/10811[#10811] and is not fixed by this change; name a `joinTable` per association to give each its own table. +Two unidirectional associations to the same class share one join table by default, named after the owner and the element class, with a key column per association (`item_role` with `item_roles_id` and `item_deny_roles_id` for `static hasMany = [roles: Role, denyRoles: Role]`). Such a shared table is now written as well: each association keeps its rows unique by its own key column and the element. A list or a map is left as it was: its join table is keyed by the index, and its elements may repeat. On most databases that is a unique key per association; on Oracle and on Db2 for Linux, UNIX and Windows, which compare the non-null columns of a partially null row in a unique key, it is one unique key over the key columns of both associations and the element. A schema created by an earlier Grails 8 release has the single unique key Hibernate made for the first association: `dbCreate` of `update` adds the second one, and on Oracle and Db2 for Linux, UNIX and Windows a migration has to repla ce that key with the wider one. Review Comment: Two small things in this paragraph. The list-and-map sentence sits between "each association keeps its rows unique by its own key column and the element" and "On most databases that is a unique key per association", so "that" now reads as the list's key. Moving it after the Oracle and Db2 sentence fixes the reading, and a bag belongs in it too, since it is also left without the key. Also, GORM now sets `hibernate.query.sqm.translator` unless the application sets it. The next paragraph says that an application with its own `hibernate.persister.resolver` keeps it and loses the #16691 fix. A matching sentence for the translator would say the same about `is empty` on associations that share a key column (#16748). -- 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]
