Skip to content

Commit 54ea208

Browse files
borinquenkidclaude
borinquenkid
and
claude
authored
fix(grails-data-hibernate7): make type: 'text' produce an unbounded column (#16020)
* fix(grails-data-hibernate7): make type: 'text' produce an unbounded column 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> * fix(grails-data-hibernate7): resolve type 'text' length via dialect-neutral 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> * docs: clarify type: 'text' resolves to the dialect's own unbounded column 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> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent f3413b4 commit 54ea208

9 files changed

Lines changed: 384 additions & 31 deletions

File tree

‎grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/ColumnBinder.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,16 @@ namingStrategy, new DefaultColumnNameFetcher(namingStrategy), new BackticksRemov
8484
* @param column The column to bind
8585
* @param path the path
8686
* @param table The table name
87+
* @param typeName the property's resolved Hibernate type name
8788
*/
8889
public void bindColumn(
8990
HibernatePersistentProperty property,
9091
HibernatePersistentProperty parentProperty,
9192
Column column,
9293
ColumnConfig cc,
9394
String path,
94-
Table table) {
95+
Table table,
96+
String typeName) {
9597

9698
if (cc != null) {
9799
column.setComment(cc.getComment());
@@ -116,7 +118,7 @@ public void bindColumn(
116118
Class<?> type = property.getType();
117119
if (type != null && (String.class.isAssignableFrom(type) || byte[].class.isAssignableFrom(type))) {
118120
PropertyConfig mappedForm = property.getHibernateMappedForm();
119-
stringColumnConstraintsBinder.bindStringColumnConstraints(column, mappedForm);
121+
stringColumnConstraintsBinder.bindStringColumnConstraints(column, mappedForm, typeName);
120122
} else if (type != null && Number.class.isAssignableFrom(type)) {
121123
PropertyConfig mappedForm = property.getHibernateMappedForm();
122124
numericColumnConstraintsBinder.bindNumericColumnConstraints(column, cc, mappedForm, type);

‎grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/SimpleValueBinder.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public SimpleValue bindSimpleValue(
8484
String path) {
8585

8686
PropertyConfig propertyConfig = property.getHibernateMappedForm();
87-
simpleValue.setTypeName(property.getTypeName(simpleValue));
87+
String typeName = property.getTypeName(simpleValue);
88+
simpleValue.setTypeName(typeName);
8889
simpleValue.setTypeParameters(property.getTypeParameters(simpleValue));
8990

9091
if (propertyConfig.isDerived() && !(property instanceof TenantId)) {
@@ -100,7 +101,7 @@ public SimpleValue bindSimpleValue(
100101
.forEach(cc -> {
101102
Column column = new Column();
102103
columnConfigToColumnBinder.bindColumnConfigToColumn(column, cc, propertyConfig);
103-
columnBinder.bindColumn(property, parentProperty, column, cc, path, table);
104+
columnBinder.bindColumn(property, parentProperty, column, cc, path, table, typeName);
104105
if (simpleValue instanceof DependantValue) {
105106
column.setNullable(true);
106107
}

‎grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/cfg/domainbinding/binder/StringColumnConstraintsBinder.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,39 @@
2121
import java.util.Objects;
2222
import java.util.Optional;
2323

24+
import org.hibernate.Length;
2425
import org.hibernate.mapping.Column;
2526

2627
import org.grails.datastore.mapping.config.Property;
2728

2829
public class StringColumnConstraintsBinder {
2930

30-
public void bindStringColumnConstraints(Column column, Property mappedForm) {
31+
/**
32+
* Binds a String/byte[] column's length from the property's {@code maxSize}/{@code inList}
33+
* constraints. When neither is present and the resolved Hibernate type name is {@code text},
34+
* the column is left unbounded via Hibernate's capacity-dependent DDL type mechanism -
35+
* {@code Length.LONG32} is the documented way to obtain each dialect's native unbounded string
36+
* type (text/longtext/varchar(max)/clob) instead of a bounded VARCHAR - see GH-16010.
37+
*
38+
* @param column the column to bind the length onto
39+
* @param mappedForm the property's constraints (maxSize/inList)
40+
* @param typeName the resolved Hibernate type name, or {@code null} if not relevant
41+
*/
42+
public void bindStringColumnConstraints(Column column, Property mappedForm, String typeName) {
3143
Integer number = Optional.ofNullable(mappedForm.getMaxSize())
3244
.map(Number::intValue)
3345
.orElse(getMax(mappedForm).orElse(0));
3446
if (number > 0) {
3547
column.setLength(number);
48+
} else if (isUnboundedTextType(typeName)) {
49+
column.setLength(Length.LONG32);
3650
}
3751
}
3852

53+
private static boolean isUnboundedTextType(String typeName) {
54+
return "text".equalsIgnoreCase(typeName);
55+
}
56+
3957
private Optional<Integer> getMax(Property mappedForm) {
4058
return Optional.ofNullable(mappedForm.getInList()).flatMap(list -> list.stream()
4159
.map(this::parseInt)

‎grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/GormTextTypeColumnIntegrationSpec.groovy‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.grails.orm.hibernate
20+
21+
import grails.gorm.annotation.Entity
22+
import grails.gorm.hibernate.HibernateEntity
23+
import grails.gorm.tests.HibernateGormDatastoreSpec
24+
import org.testcontainers.mariadb.MariaDBContainer
25+
import org.testcontainers.mysql.MySQLContainer
26+
import org.testcontainers.postgresql.PostgreSQLContainer
27+
import org.testcontainers.spock.Testcontainers
28+
import spock.lang.Requires
29+
import spock.lang.Shared
30+
31+
/**
32+
* Reproduces https://github.com/apache/grails-core/issues/16010 across every externally-run
33+
* dialect this module tests against (see {@link grails.gorm.tests.RLikeHibernate7Spec} for the
34+
* same H2/Postgres/MySQL/MariaDB precedent): a property mapped with {@code type: 'text'} must
35+
* produce a genuinely unbounded column, not a bounded {@code varchar(n)}/{@code character
36+
* varying(n)} that can fail to accommodate existing data on schema update. Oracle is
37+
* intentionally excluded - its Testcontainers image is too flaky in CI to gate this spec on.
38+
* H2 coverage lives separately in {@link GormTextTypeColumnLengthSpec}, which needs no container
39+
* and so still runs when Docker (and therefore this whole spec) is unavailable.
40+
*/
41+
@Testcontainers
42+
@Requires({ isDockerAvailable() })
43+
class GormTextTypeColumnIntegrationSpec extends HibernateGormDatastoreSpec {
44+
45+
@Shared PostgreSQLContainer postgres = new PostgreSQLContainer("postgres:16")
46+
@Shared MySQLContainer mysql = new MySQLContainer("mysql:8.0")
47+
@Shared MariaDBContainer mariadb = new MariaDBContainer("mariadb:10.11")
48+
49+
void setupSpec() {
50+
manager.registerDomainClasses(TextTypeMessage)
51+
}
52+
53+
void "a property mapped with type 'text' produces an unbounded column on #db"() {
54+
given:
55+
if (!container.isRunning()) {
56+
container.start()
57+
}
58+
// Ensure a completely fresh datastore per dialect, as in RLikeHibernate7Spec.
59+
manager.destroy()
60+
manager.grailsConfig = [
61+
'dataSource.url' : container.jdbcUrl,
62+
'dataSource.driverClassName' : container.driverClassName,
63+
'dataSource.username' : container.username,
64+
'dataSource.password' : container.password,
65+
'dataSource.dbCreate' : 'create-drop',
66+
'hibernate.hbm2ddl.auto' : 'create',
67+
]
68+
// 'hibernate.dialect' is intentionally omitted - Hibernate 7 auto-detects it from
69+
// JDBC metadata, avoiding a hardcoded dialect string per database.
70+
manager.setup(this.class)
71+
72+
when:
73+
Map<String, Object> column
74+
datastore.dataSource.connection.withCloseable { conn ->
75+
conn.createStatement().withCloseable { stmt ->
76+
stmt.executeQuery('''
77+
select character_maximum_length
78+
from information_schema.columns
79+
where upper(table_name) = 'TEXT_TYPE_MESSAGE' and upper(column_name) = 'BODY'
80+
'''.stripIndent()).with { rs ->
81+
rs.next()
82+
column = [maxLength: rs.getObject('character_maximum_length')]
83+
}
84+
}
85+
}
86+
87+
then: 'no small bounded length is reported - a regression would report 32600 (Length.LONG)'
88+
column.maxLength == null || (column.maxLength as long) > 1_000_000L
89+
90+
where:
91+
db | container
92+
"Postgres" | postgres
93+
"MySQL" | mysql
94+
"MariaDB" | mariadb
95+
}
96+
}
97+
98+
@Entity
99+
class TextTypeMessage implements HibernateEntity<TextTypeMessage> {
100+
String body
101+
102+
static mapping = {
103+
body type: 'text'
104+
}
105+
}

‎grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/GormTextTypeColumnLengthSpec.groovy‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.grails.orm.hibernate
20+
21+
import grails.gorm.annotation.Entity
22+
import grails.gorm.hibernate.HibernateEntity
23+
import grails.gorm.tests.HibernateGormDatastoreSpec
24+
import org.hibernate.Length
25+
import org.hibernate.mapping.PersistentClass
26+
27+
/**
28+
* Covers https://github.com/apache/grails-core/issues/16010 against the default H2 datastore
29+
* used by the rest of the test suite, so the {@code type: 'text'} column-length behaviour is
30+
* exercised even when Docker (and so {@link GormTextTypeColumnIntegrationSpec}'s Postgres/MySQL/
31+
* MariaDB Testcontainers) is unavailable.
32+
*/
33+
class GormTextTypeColumnLengthSpec extends HibernateGormDatastoreSpec {
34+
35+
void setupSpec() {
36+
manager.registerDomainClasses(UnboundedTextTypeMessage, BoundedTextTypeMessage)
37+
}
38+
39+
void "a property mapped with type 'text' and no explicit length is bound to Length.LONG32"() {
40+
when:
41+
PersistentClass persistentClass = datastore.getMetadata().getEntityBinding(UnboundedTextTypeMessage.name)
42+
def column = persistentClass.getProperty('body').getColumns().first()
43+
44+
then:
45+
column.getLength() == Length.LONG32 as Long
46+
}
47+
48+
void "a property mapped with type 'text' and an explicit maxSize keeps the bounded length"() {
49+
when:
50+
PersistentClass persistentClass = datastore.getMetadata().getEntityBinding(BoundedTextTypeMessage.name)
51+
def column = persistentClass.getProperty('body').getColumns().first()
52+
53+
then:
54+
column.getLength() == 500L
55+
}
56+
57+
void "a property mapped with type 'text' and no explicit length produces an unbounded H2 CLOB column"() {
58+
when:
59+
Map<String, Object> column
60+
datastore.dataSource.connection.withCloseable { conn ->
61+
conn.createStatement().withCloseable { stmt ->
62+
stmt.executeQuery('''
63+
select data_type, character_maximum_length
64+
from information_schema.columns
65+
where table_name = 'UNBOUNDED_TEXT_TYPE_MESSAGE' and column_name = 'BODY'
66+
'''.stripIndent()).with { rs ->
67+
rs.next()
68+
column = [dataType: rs.getString('data_type'), maxLength: rs.getObject('character_maximum_length')]
69+
}
70+
}
71+
}
72+
73+
then:
74+
column.dataType == 'CHARACTER LARGE OBJECT'
75+
column.maxLength == Long.MAX_VALUE
76+
}
77+
}
78+
79+
@Entity
80+
class UnboundedTextTypeMessage implements HibernateEntity<UnboundedTextTypeMessage> {
81+
String body
82+
83+
static mapping = {
84+
body type: 'text'
85+
}
86+
}
87+
88+
@Entity
89+
class BoundedTextTypeMessage implements HibernateEntity<BoundedTextTypeMessage> {
90+
String body
91+
92+
static constraints = {
93+
body maxSize: 500
94+
}
95+
96+
static mapping = {
97+
body type: 'text'
98+
}
99+
}

0 commit comments

Comments
 (0)