jamesfredley commented on issue #15736:
URL: https://github.com/apache/grails-core/issues/15736#issuecomment-4724216851

   I dug into this and can confirm the root cause. The custom `NamingStrategy` 
is honored for entity **table** names but ignored for the foreign-key 
**column** names generated inside `hasMany`/many-to-many join tables.
   
   The naming strategy exposes two separate methods, and the binder calls a 
different one depending on the path.
   
   **Entity table names (works).** In `GrailsDomainBinder.getTableName()` the 
binder uses the *simple* class name and routes through `classToTableName()`:
   
   ```java
   String shortName = domainClass.getJavaClass().getSimpleName();   // "TBook"
   tableName = getNamingStrategy(...).classToTableName(shortName);  // -> 
"book" (T stripped)
   ```
   
   **Join-table FK columns (broken).** The reproduction declares `hasMany` on 
both sides, so it's a many-to-many and binding flows through 
`getForeignKeyForPropertyDomainClass()`:
   
   ```java
   final String propertyName = 
NameUtils.decapitalize(property.getOwner().getName());
   return namingStrategy.propertyToColumnName(propertyName) + 
FOREIGN_KEY_SUFFIX;  // -> "...tbook_id"
   ```
   
   This path calls `propertyToColumnName()`, **not** `classToTableName()`. 
Since `TNamingStrategy` overrides only `classToTableName()`, the `T`-stripping 
logic is never invoked for FK columns, so the prefix survives. The plain 
(non-m2m) `hasMany` path has the same defect where it builds the element column 
name.
   
   There is also a secondary inconsistency: the table path feeds 
`getSimpleName()` while these FK paths feed `getName()` (fully-qualified), so 
even the input to the strategy differs between the two code paths.
   
   That mismatch is exactly why the `static mapping` `joinTable` override is 
currently required as a workaround.
   
   This lives in GORM's Hibernate binding (`GrailsDomainBinder`) and is present 
identically in both `grails-data-hibernate7` and `grails-data-hibernate5`:
   
   | Path | Method | Strategy method called | Result |
   |------|--------|------------------------|--------|
   | `getTableName()` (line 1258/1265) | uses `getSimpleName()` | 
`classToTableName()` | `T` stripped (correct) |
   | `bindCollectionWithJoinTable()` (line 770) | uses `getName()` | 
`propertyToColumnName()` | `T` retained |
   | `getForeignKeyForPropertyDomainClass()` (line 3222) | uses `getName()` | 
`propertyToColumnName()` | `T` retained |
   
   A fix would make the FK-column derivation consistent with `getTableName()` 
(derive the FK prefix from the resolved table name / 
`classToTableName(getSimpleName())`). Worth noting it's a schema-affecting 
behavior change for existing apps that rely on the current column names, so it 
should land with a test and a clear note in the release docs.
   


-- 
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]

Reply via email to