fix(grails-data-hibernate7): make type: 'text' produce an unbounded column - #16020
fix(grails-data-hibernate7): make type: 'text' produce an unbounded column#16020borinquenkid wants to merge 3 commits into
Conversation
…olumn property type: 'text' resolved through Hibernate's legacy named-type lookup to StandardBasicTypes.TEXT, whose JDBC type code is the legacy java.sql.Types.LONGVARCHAR. Dialects (e.g. Postgres) don't render that legacy code as their native unbounded text/CLOB type, falling back to a bounded VARCHAR at Hibernate's generic Length.LONG default (32600) once no explicit column length is set. On schema update, altering an existing column down to that bound fails once any row already holds more text. Bind the modern SqlTypes.LONG32VARCHAR JDBC type directly for this case instead of going through the ambiguous legacy type name, restoring the "CLOB or TEXT depending on dialect" behavior the mapping DSL docs already promise for type: 'text'. Fixes #16010 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a Grails Data Hibernate 7 mapping issue where type: 'text' incorrectly generated a bounded varchar(32600) on PostgreSQL, causing schema-update failures once existing data exceeded that limit. The change targets the Hibernate binding layer to force an unbounded/text-capable JDBC type, and adds an integration test that validates the generated column type against a real Postgres instance.
Changes:
- Special-case
type: 'text'during simple value binding to set an explicit JDBC type (SqlTypes.LONG32VARCHAR) so dialects render an unbounded TEXT/CLOB as appropriate. - Add a Testcontainers-based integration spec that asserts Postgres produces
data_type = 'text'andcharacter_maximum_length = nullfor atype: 'text'mapped property.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleValueBinder.java | Forces modern JDBC type binding for type: 'text' to avoid Postgres rendering a bounded VARCHAR. |
| grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/GormTextTypeColumnIntegrationSpec.groovy | New Postgres Testcontainers integration test asserting type: 'text' creates an unbounded text column. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16020 +/- ##
==================================================
- Coverage 51.4393% 51.4297% -0.0096%
+ Complexity 17724 17723 -1
==================================================
Files 2039 2039
Lines 95497 95507 +10
Branches 16564 16564
==================================================
- Hits 49123 49119 -4
- Misses 39076 39088 +12
- Partials 7298 7300 +2 🚀 New features to boost your workflow:
|
| // default (32600) when no explicit column length is set - see GH-16010. The modern | ||
| // SqlTypes.LONG32VARCHAR code is what dialects actually map to an unbounded type, so | ||
| // bind that directly rather than going through the ambiguous legacy type name. | ||
| basicValue.setExplicitJdbcTypeAccess( |
There was a problem hiding this comment.
Isn't this dialect specific? This silently changes the DDL of type: 'text' on H2, MySQL/MariaDB, Oracle, and SQL Server:
- H2 (the default test/dev DB): LONG32VARCHAR → character large object/clob rather than whatever LONGVARCHAR rendered before.
- Oracle: → clob instead of the legacy path.
- SQL Server: → varchar(max).
- MySQL: → longtext.
i think this is ok, but I think in hiberate5 it was only supported if the dialect supported it.
|
The change explicitly maps the 'text' type to grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleValueBinder.java |
jdaugherty
left a comment
There was a problem hiding this comment.
I was looking at hibernate 6 and I think a better solution would be to support maxSize: null. Take a look at my comment that I generated with AI after discussing this issue.
|
The direction is right, but rather than overriding the JDBC type via Why Hibernate 6 introduced exactly this mechanism as the replacement for the removed One wrinkle worth addressing at the mapping level: GORM always defaults a length for string columns (the length is never truly "unset" by the time the column is bound), so there is currently no way for a user to express "unbounded" other than smuggling it through a type name like Advantages over the explicit-JDBC-type override:
A couple of test asks either way:
|
…eutral Length.LONG32 Address PR review feedback on the type: 'text' unbounded-column fix: instead of overriding the JDBC type descriptor with SqlTypes.LONG32VARCHAR, set the column's length to Hibernate 6+'s documented Length.LONG32 sentinel and let each dialect's own capacity-dependent DDL type registry resolve the native unbounded type (text, longtext, CLOB). This composes correctly with maxSize/inList/explicit column length instead of racing them, and keeps SimpleValueBinder as a pure orchestrator - the length decision now lives in StringColumnConstraintsBinder, which already owns string column length for maxSize/inList. Extends test coverage to close the "Postgres-only" gap: adds an H2-based spec that runs without Docker so container-less CI still exercises this path, and extends the Testcontainers spec to MySQL and MariaDB (Oracle excluded, matching the flaky-in-CI precedent already established in RLikeHibernate7Spec). Reverting the fix locally confirmed MySQL/MariaDB were independently affected (TEXT capped at 65535), not just Postgres. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@jdaugherty Thanks for the review — implemented your suggested direction in 3c0350d → a8f5031. Mechanism swap: dropped Composability: confirmed Where the logic lives: moved the length decision into Test coverage, per your ask:
One thing from your comment I did not implement: the broader idea of a first-class "unset length = unbounded" knob usable by any string type, not just Full |
jdaugherty
left a comment
There was a problem hiding this comment.
I think this solution is acceptable but it does diverge from Hibernate. Making maxSize nullable or be the larger size like hibernate did in 6 is probably a more correct solution. We effectively have added support to all databases for the text type though. It seems like we should document this divergence at a minimum.
|
@borinquenkid let's document this so we can merge this change for now? I do think a follow-up ticket needs filed, adding 'text' type on a databaes that doesn't support it really should error and hibernate has changed the semantics in later versions - we really should match their binding style or at least make it clear when gorm is "calculating" something for us vs how to override the underlying mappings. |
…lumn Addresses review feedback on #16020 asking to document that GORM computes the concrete SQL type (text/longtext/CLOB) per dialect rather than emitting a literal "text" type, and that every Hibernate-shipped Dialect defines this mapping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ All tests passed ✅🏷️ Commit: a5f651e Learn more about TestLens at testlens.app. |
Summary
Fixes #16010 — a GORM property mapped with
type: 'text'was producing a boundedvarchar(32600)column on Postgres instead of a genuine unboundedtextcolumn, causingCommandAcceptanceException: value too long for type character varying(32600)on schema update once existing data exceeded that length.Root cause:
type: 'text'resolves through Hibernate's legacy named-type lookup toStandardBasicTypes.TEXT, whose JDBC type code is the legacyjava.sql.Types.LONGVARCHAR. Dialects (e.g. Postgres) don't render that legacy code as their native unboundedtext/CLOB type, falling back instead to a boundedVARCHARat Hibernate's genericLength.LONGdefault (32600) once no explicit column length is set (GORM only sets an explicit length when amaxSize/inListvalidation constraint exists).Fix: in
SimpleValueBinder.bindSimpleValue(), when the resolved type name is"text", bind the modernSqlTypes.LONG32VARCHARJDBC type directly viaBasicValue.setExplicitJdbcTypeAccess(...)instead of going through the ambiguous legacy type name — restoring the "CLOB or TEXT depending on database dialect" behavior the mapping DSL reference docs (type.adoc) already promise fortype: 'text'.Test plan
GormTextTypeColumnIntegrationSpec(real Postgres 16 via Testcontainers, following the module's existingHibernateDatastoreIntegrationSpecpattern): confirmed RED against unmodified code (character varying/32600, reproducing the bug exactly), confirmed GREEN after the fix (text/unbounded)grails-data-hibernate7-coresuite: 3007 tests, 0 failures, 24 pre-existing skipscodenarcMain/codenarcTest/checkstyleMain/checkstyleTest: cleanCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com