Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
import org.grails.datastore.mapping.model.PersistentEntity;
import org.grails.datastore.mapping.model.PersistentProperty;
import org.grails.orm.hibernate.cfg.Mapping;
import org.grails.orm.hibernate.cfg.PersistentEntityNamingStrategy;
import org.grails.orm.hibernate.cfg.PropertyConfig;
import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover;

/**
* Common interface for all Hibernate association properties (both ToOne and ToMany). Extends {@link
Expand Down Expand Up @@ -80,6 +82,15 @@ default String getReferencedEntityName() {
return getHibernateAssociatedEntity().getName();
}

default String resolveAssociatedEntityTableName(PersistentEntityNamingStrategy namingStrategy) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the table name verbatim, and the two call sites then treat it differently: joinTableColumName passes it through BackticksRemover, while resolveJoinTableForeignKeyColumnName concatenates _id onto it directly. TableForManyCalculator.calculateTableForMany also strips backticks from getTableName(...), because backtick-quoting a reserved word in table is supported and used (e.g. grails/gorm/tests/multitenancy/User maps table 'user').

With a quoted table on the far side of a unidirectional hasMany, the FK column name is now malformed:

@Entity class ProbeQuoted { String label
    static mapping = { table '`user`' } }

@Entity class ProbeShelf { String label
    static hasMany = [quoted: ProbeQuoted] }
Error executing DDL "create table probe_shelf_user (`user`_id bigint, probe_shelf_quoted_id bigint, unique (probe_shelf_quoted_id, `user`_id))"
  via JDBC [Unknown data type: "_ID"]

On 8.0.x the same mapping produces probe_shelf_user(probe_quoted_id, probe_shelf_quoted_id). Without hibernate.hbm2ddl.halt_on_error the statement fails silently and the join table is simply missing from the generated schema, which makes it an easy one to ship unnoticed.

Stripping backticks here (or at the resolveJoinTableForeignKeyColumnName call site, matching joinTableColumName) fixes it. A test with a backtick-quoted table mapping would be worth adding alongside the two new cases.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 546fea4 (pushed before your review, sorry for the noise) — stripped backticks once at the source in HibernateAssociation#resolveAssociatedEntityTableName, which both joinTableColumName and resolveJoinTableForeignKeyColumnName go through, so it's no longer left to each caller. Added "resolveJoinTableForeignKeyColumnName strips backticks from a backtick-quoted associated entity table name" reproducing your table '\user`'-shaped repro (HTMPQuotedTableAuthor/HTMPQuotedTableBook` in the spec).

// Every caller of this method uses the result as a column-identifier fragment (a join-table foreign-key
// or element column name), never as a literal, quotable SQL identifier - so the Groovy backtick-quoting
// convention is always invalid there and must be stripped once at the source, rather than trusted to
// each caller (a prior bug left one caller emitting a malformed column like `quoted_table`_id).
return new BackticksRemover().apply(
getHibernateAssociatedEntity().getHibernateRootEntity().getTableName(namingStrategy));
}

@Override
default void validateAssociation() {
if (getUserType() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,14 +208,16 @@ default String getMapElementName(PersistentEntityNamingStrategy namingStrategy)
IndexedCollection.DEFAULT_ELEMENT_COLUMN_NAME);
}

/**
* Only reached for a unidirectional {@code hasMany} join table (via {@code CollectionWithJoinTableBinder}).
* A bidirectional many-to-many join table's foreign-key columns instead go through
* {@code DefaultColumnNameFetcher#resolveForeignKeyForPropertyDomainClass}, unaffected by this method.
*/
default String resolveJoinTableForeignKeyColumnName(PersistentEntityNamingStrategy namingStrategy) {
return ofNullable(getHibernateMappedForm())
.map(PropertyConfig::getJoinTableColumnConfig)
.map(ColumnConfig::getName)
.orElseGet(() -> namingStrategy.resolveColumnName(getHibernateAssociatedEntity()
.getHibernateRootEntity()
.getJavaClass()
.getSimpleName()) +
.orElseGet(() -> resolveAssociatedEntityTableName(namingStrategy) +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveJoinTableForeignKeyColumnName only runs for unidirectional hasMany join tables, so a bidirectional many-to-many is not affected by this change.

CollectionSecondPassBinder sends a bidirectional many-to-many element to ManyToOneElementBinderManyToOneBinderSimpleValueBinderDefaultColumnNameFetcher, which for a HibernateManyToManyProperty returns resolveForeignKeyForPropertyDomainClass(...) — still the decapitalized class simple name run through resolveColumnName. The only production call site of the method changed here is CollectionWithJoinTableBinder, reached from UnidirectionalOneToManyBinder.

I compared the generated H2 schema on this branch against the merge base with these domain classes:

@Entity class ProbeAuthor { String name
    static hasMany = [books: ProbeBook]
    static mapping = { table 'writer' } }

@Entity class ProbeBook { String title
    static belongsTo = ProbeAuthor
    static hasMany = [authors: ProbeAuthor]
    static mapping = { table 'catalog_book' } }

@Entity class ProbeShelf { String label
    static hasMany = [shelved: ProbeBook] }   // unidirectional
join table 8.0.x this branch
writer_books (bidirectional many-to-many) probe_author_id, probe_book_id unchanged
probe_shelf_catalog_book (unidirectional) probe_book_id, probe_shelf_shelved_id catalog_book_id, probe_shelf_shelved_id

Two consequences worth deciding on explicitly:

Could we either extend the resolution to DefaultColumnNameFetcher#getDefaultColumnName / resolveForeignKeyForPropertyDomainClass so both sides of a join table agree, or keep the code change as-is and narrow the documentation to the unidirectional case it actually covers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed repro — went with your second option: kept the code narrowly scoped to what it actually fixes (the associated-entity FK column of a unidirectional hasMany, via CollectionWithJoinTableBinder) rather than extending it into DefaultColumnNameFetcher/many-to-many, which would be a materially larger change than #15736 asked for. Instead, re-scoped the docs to describe exactly that in 4da31cc: upgrading80x.adoc §26.9 now leads with the unidirectional-only scope, replaces the many-to-many example with a genuinely unidirectional one (Shelf hasMany books, no belongsTo), and notes the owner-side column (shelf_id) is unchanged. Also added a doc comment on resolveJoinTableForeignKeyColumnName itself recording the scope and pointing at DefaultColumnNameFetcher#resolveForeignKeyForPropertyDomainClass as the unaffected many-to-many path, and a closing paragraph noting the resolveTableNameresolveColumnName property-prefix change you flagged for basic/enum collections.

GrailsDomainBinder.FOREIGN_KEY_SUFFIX);
}

Expand All @@ -227,8 +229,11 @@ default String joinTableColumName(PersistentEntityNamingStrategy namingStrategy)
if (present) {
columnName = joinColumnMappingOptional.get().getName();
} else {
// Both callers of joinTableColumName (BasicCollectionElementBinder, EnumTypeBinder) operate on
// a HibernateBasicProperty, so referencedType is always the collection's basic element type here,
// never an associated entity - resolveAssociatedEntityTableName does not apply to this path.
var clazz = namingStrategy.resolveColumnName(referencedType.getName());
var prop = namingStrategy.resolveTableName(getName());
var prop = namingStrategy.resolveColumnName(getName());
Comment thread
matrei marked this conversation as resolved.
columnName = referencedType.isEnum() ?
clazz :
new BackticksRemover().apply(prop) + UNDERSCORE + new BackticksRemover().apply(clazz);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ package org.grails.orm.hibernate.cfg.domainbinding.hibernate
import grails.gorm.annotation.Entity
import grails.gorm.tests.HibernateGormDatastoreSpec
import org.hibernate.MappingException
import org.hibernate.boot.model.naming.Identifier
import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment
import org.grails.datastore.mapping.model.PersistentProperty
import org.grails.datastore.mapping.model.PersistentEntity
import org.grails.datastore.mapping.model.PropertyMapping
import org.grails.datastore.mapping.model.ClassMapping
import org.grails.datastore.mapping.reflect.EntityReflector
import org.grails.orm.hibernate.cfg.PropertyConfig
import org.grails.orm.hibernate.cfg.Mapping
import org.grails.orm.hibernate.cfg.domainbinding.util.NamingStrategyWrapper

class HibernateToManyPropertySpec extends HibernateGormDatastoreSpec {

Expand Down Expand Up @@ -63,6 +67,37 @@ class HibernateToManyPropertySpec extends HibernateGormDatastoreSpec {
columnName == "custom_book_fk"
}

void "resolveJoinTableForeignKeyColumnName removes a domain prefix through a physical naming strategy"() {
given:
def property = createTestHibernateToManyProperty(HTMPAuthor, "books")
def namingStrategy = new NamingStrategyWrapper(
new HTMPPrefixRemovingPhysicalNamingStrategy(), getGrailsDomainBinder().jdbcEnvironment)
hibernateFirstPass()

expect:
property.resolveJoinTableForeignKeyColumnName(namingStrategy) == "book_id"
}

void "resolveJoinTableForeignKeyColumnName uses the associated entity explicit table mapping"() {
given:
def property = createTestHibernateToManyProperty(HTMPMappedTableAuthor, "books")
def namingStrategy = getGrailsDomainBinder().namingStrategy
hibernateFirstPass()

expect:
property.resolveJoinTableForeignKeyColumnName(namingStrategy) == "htmp_book_id"
}

void "resolveJoinTableForeignKeyColumnName strips backticks from a backtick-quoted associated entity table name"() {
given:
def property = createTestHibernateToManyProperty(HTMPQuotedTableAuthor, "books")
def namingStrategy = getGrailsDomainBinder().namingStrategy
hibernateFirstPass()

expect:
property.resolveJoinTableForeignKeyColumnName(namingStrategy) == "htmp_quoted_book_id"
}

void "isAssociationColumnNullable returns false for ManyToMany"() {
given: "Register only entities for this specific test"
createPersistentEntity(HTMPCourse) // Course is needed because Student refers to it
Expand Down Expand Up @@ -351,6 +386,17 @@ class HibernateToManyPropertySpec extends HibernateGormDatastoreSpec {
property.joinTableColumName(namingStrategy) != null
}

void "joinTableColumName resolves the property prefix through column naming rather than table naming"() {
given: "a physical naming strategy where column and table transformation rules diverge for 'tags'"
def property = createTestHibernateToManyProperty(HTMPOwnerString, "tags")
def namingStrategy = new NamingStrategyWrapper(
new HTMPColumnMarkingPhysicalNamingStrategy(), getGrailsDomainBinder().jdbcEnvironment)
hibernateFirstPass()

expect: "the property prefix carries the column-naming marker; the unmarked form would mean the old resolveTableName() path ran instead"
property.joinTableColumName(namingStrategy).startsWith("tags_as_column_")
}

void "joinTableColumName returns derived column name for enum collection"() {
given:
def property = createTestHibernateToManyProperty(HTMPEntityWithEnum, "statuses")
Expand Down Expand Up @@ -581,6 +627,60 @@ class HTMPBook {
String title
}

@Entity
class HTMPMappedTableBook {
Long id
String title

static mapping = {
table 'htmp_book'
}
}

@Entity
class HTMPMappedTableAuthor {
Long id
String name
static hasMany = [books: HTMPMappedTableBook]
}

@Entity
class HTMPQuotedTableBook {
Long id
String title

static mapping = {
table '`htmp_quoted_book`'
}
}

@Entity
class HTMPQuotedTableAuthor {
Long id
String name
static hasMany = [books: HTMPQuotedTableBook]
}

class HTMPPrefixRemovingPhysicalNamingStrategy extends PhysicalNamingStrategyStandardImpl {

@Override
Identifier toPhysicalTableName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
logicalName.text == HTMPBook.simpleName ?
Identifier.toIdentifier('book') :
super.toPhysicalTableName(logicalName, jdbcEnvironment)
}
}

class HTMPColumnMarkingPhysicalNamingStrategy extends PhysicalNamingStrategyStandardImpl {

@Override
Identifier toPhysicalColumnName(Identifier logicalName, JdbcEnvironment jdbcEnvironment) {
logicalName.text == 'tags' ?
Identifier.toIdentifier('tags_as_column') :
super.toPhysicalColumnName(logicalName, jdbcEnvironment)
}
}

@Entity
class HTMPAuthor {
Long id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ class UpperCaseNamingStrategy implements PhysicalNamingStrategy {
----

TIP: Individual column or table names set explicitly in the `mapping` block always take precedence over what the naming strategy would produce.

For a *unidirectional* `hasMany` (a collection with no `belongsTo` or reciprocal `hasMany` on the other side), the default foreign-key column that references the associated entity is derived from that entity's physical table name. Consequently, a custom strategy that changes a domain table name also changes that column. For example, given `static hasMany = [books: TBook]`, if the strategy maps `TBook` to the table `book`, the default foreign-key column is `book_id`, not `tbook_id`.

This does not apply to a *bidirectional* many-to-many association: both of its join-table foreign-key columns are still derived from the class names, regardless of any `table` mapping or naming strategy. Applications upgrading from an earlier GORM version should account for the unidirectional case's schema change, or configure the join-table columns explicitly in the `mapping` block. See https://grails.apache.org/docs/latest/guide/single.html#_join_table_foreign_key_column_names[Join-Table Foreign-Key Column Names] in the upgrade guide for details and a migration example.
54 changes: 54 additions & 0 deletions grails-doc/src/en/guide/upgrading/upgrading80x.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,60 @@ GORM's `createCriteria()` and `withCriteria()` DSL are implemented on top of the

*`javax.persistence` → `jakarta.persistence`*: This migration was already required for Grails 7; Grails 8 continues to require `jakarta.*`.

[[_join_table_foreign_key_column_names]]
===== 26.9 Join-Table Foreign-Key Column Names

This change affects only a *unidirectional* `hasMany` — a collection with no `belongsTo` and no reciprocal `hasMany` on the other side. A *bidirectional* many-to-many association (both sides declare `hasMany`, or one side uses `belongsTo`) is **not** affected: both of its join-table foreign-key columns are still derived from the class names, identical to Grails 7.

In Grails 7 (Hibernate 5), the default foreign-key column that referenced the associated entity in a unidirectional `hasMany` join table was derived from the simple name of that entity's domain class, after applying the column naming strategy.
For example, the domain class `Book` produced the foreign-key column `book_id`, even when its physical table was mapped to a different name.

In Grails 8 (Hibernate 7), that foreign-key column is instead derived from the physical table name of the associated domain class.
The physical name includes an explicit `table` mapping and any transformation made by a custom physical naming strategy.
For example, given the following mapping:

[source,groovy]
----
class Book {
static mapping = {
table 'catalog_book'
}
}

class Shelf {
String label

static hasMany = [books: Book] // unidirectional: no belongsTo, no reciprocal hasMany
}
----

Grails 7 used `book_id` by default in the `shelf_books` join table, whereas Grails 8 uses `catalog_book_id`. The other column in that same join table (`shelf_id`, derived from the owning `Shelf` class) is unchanged.
This is a breaking schema change for an existing database if its join table still uses the Grails 7 column name.

To keep the existing schema unchanged, configure the join table and its foreign-key column explicitly in the `static mapping` block:

[source,groovy]
----
class Shelf {
String label

static hasMany = [books: Book]

static mapping = {
books joinTable: [
name: 'shelf_books',
key: 'shelf_id',
column: 'book_id'
]
}
}
----

Replace `shelf_books`, `shelf_id`, and `book_id` with the table and column names already used by your database.
Declaring all three names prevents the naming strategy from changing the mapping during the upgrade.

Separately, the element column of a *basic or enum collection* (e.g. `static hasMany = [items: String]`) is now resolved through the naming strategy's *column* naming rules instead of its *table* naming rules for the property-name prefix (e.g. `items_value`). Most naming strategies apply the same transformation to both, so this only matters if your custom `PhysicalNamingStrategy` implements `toPhysicalColumnName` and `toPhysicalTableName` differently.

==== 27. GORM Properties Are Nullable by Default

Grails 8 changes the validation default so that an *unconstrained persistent (domain) property is nullable by default* rather than required.
Expand Down
Loading