borinquenkid commented on code in PR #16583:
URL: https://github.com/apache/grails-core/pull/16583#discussion_r4420305070
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/core/Datastore.java:
##########
@@ -86,6 +87,47 @@ public interface Datastore extends ServiceRegistry {
*/
boolean isSchemaless();
+ /**
+ * How the {@code unique} constraint treats a {@code null} in a composite
key for the entities of this datastore:
+ * the {@code grails.gorm.unique.nulls} setting, whose default {@code
database} reads
+ * {@link #getDatabaseUniqueNulls()}. Never {@link UniqueNulls#DATABASE}.
+ *
+ * @return the reading
+ */
+ default UniqueNulls getUniqueNulls() {
+ return getDatabaseUniqueNulls();
+ }
+
+ /**
+ * How the unique index of the database this datastore keeps treats a
{@code null} in a composite key: {@link UniqueNulls#SKIP}
+ * where nulls are distinct, {@link UniqueNulls#VALUE} where they are
compared. A datastore that keeps no such index,
+ * or does not know the database, answers {@link UniqueNulls#VALUE}.
+ *
+ * @return {@link UniqueNulls#SKIP} or {@link UniqueNulls#VALUE}
+ */
+ default UniqueNulls getDatabaseUniqueNulls() {
+ return UniqueNulls.VALUE;
+ }
+
+ /**
+ * The configured {@link UniqueNulls} reading from settings, before
resolving {@link UniqueNulls#DATABASE}.
+ *
+ * @return the configured mode, or {@code null} if unconfigured
+ */
+ default UniqueNulls getConfiguredUniqueNulls() {
+ return null;
+ }
+
+ /**
+ * Whether the unique index of the database this datastore keeps omits a
key that is {@code null} in every
+ * column, so that two rows whose unique key is entirely null do not
conflict.
+ *
+ * @return {@code true} if entirely-null unique keys are exempt from
uniqueness checks under the database reading
+ */
+ default boolean isEntirelyNullKeyExempt() {
Review Comment:
Updated the javadoc of `isEntirelyNullKeyExempt()` to state that the `true`
default represents the absence of a unique index for datastores such as
`MongoDatastore`, `Neo4jDatastore`, and `SimpleMapDatastore`.
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/UniqueNullsHibernate7Spec.groovy:
##########
@@ -0,0 +1,258 @@
+/*
+ * 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 grails.gorm.tests
+
+import java.time.Duration
+
+import org.springframework.dao.DataIntegrityViolationException
+import org.testcontainers.mariadb.MariaDBContainer
+import org.testcontainers.mysql.MySQLContainer
+import org.testcontainers.oracle.OracleContainer
+import org.testcontainers.postgresql.PostgreSQLContainer
+import org.testcontainers.spock.Testcontainers
+import spock.lang.Requires
+import spock.lang.Shared
+import spock.lang.Unroll
+
+import grails.gorm.annotation.Entity
+import org.grails.datastore.mapping.config.UniqueNulls
+
+/**
+ * The {@code unique} constraint reads a null in a composite key the way the
unique index of the database does, unless it
+ * is told otherwise: on every database the validation of a key whose group is
null agrees with whether the database
+ * accepts the same row twice.
+ */
+@Testcontainers
+@Requires({ isDockerAvailable() })
+class UniqueNullsHibernate7Spec extends HibernateGormDatastoreSpec {
+
+ @Shared postgres = new PostgreSQLContainer('postgres:16')
+ @Shared mysql = new MySQLContainer('mysql:8.0')
+ @Shared mariadb = new MariaDBContainer('mariadb:10.11')
+ @Shared oracle = new OracleContainer('gvenzl/oracle-free:slim-faststart')
+ .withStartupTimeout(Duration.ofMinutes(3))
+
+ void setupSpec() {
+ manager.registerDomainClasses(UnqLanguage, UnqLanguageValue, UnqEmail)
+ }
+
+ @Unroll
+ void 'on #db the validation reads nulls as #reading, as the unique index
does'() {
+ given: 'the datastore on this database, with the schema created for it'
+ if (container != null && !container.isRunning()) {
+ container.start()
+ }
+ manager.destroy()
+ manager.grailsConfig = [
+ 'dataSource.url' : container?.jdbcUrl ?:
"jdbc:h2:mem:uniqueNulls${db};LOCK_TIMEOUT=10000".toString(),
+ 'dataSource.driverClassName': container?.driverClassName ?:
'org.h2.Driver',
+ 'dataSource.username' : container?.username ?: 'sa',
+ 'dataSource.password' : container?.password ?: '',
+ 'dataSource.dbCreate' : 'create-drop'
+ ]
+ manager.setup(this.class)
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: 'en', country: null).save(validate:
false, flush: true)
+ }
+
+ when: 'the same key is validated'
+ boolean validationRejects = UnqLanguage.withNewSession {
+ UnqLanguage.withNewTransaction {
+ !new UnqLanguage(language: 'en', country: null).validate()
+ }
+ }
+
+ and: 'and the database is asked to take the same row twice'
+ boolean indexRejects = false
+ try {
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: 'en', country: null).save(validate:
false, flush: true)
+ }
+ } catch (DataIntegrityViolationException ignored) {
+ indexRejects = true
+ }
+
+ then: 'the datastore says what the database does, and the validation
agrees with the index'
+ manager.session.datastore.databaseUniqueNulls == reading
+ manager.session.datastore.uniqueNulls == reading
+ validationRejects == indexRejects
+ indexRejects == (reading == UniqueNulls.VALUE)
+
+ where:
+ db | container || reading
+ 'H2' | null || UniqueNulls.SKIP
+ 'Postgres' | postgres || UniqueNulls.SKIP
+ 'MySQL' | mysql || UniqueNulls.SKIP
+ 'MariaDB' | mariadb || UniqueNulls.SKIP
+ 'Oracle' | oracle || UniqueNulls.VALUE
+ }
+
+ @Unroll
+ void 'the setting #setting overrides the database on H2, and a constraint
option overrides the setting'() {
+ given:
+ manager.destroy()
+ Map config = [
+ 'dataSource.url' :
'jdbc:h2:mem:uniqueNullsSetting;LOCK_TIMEOUT=10000',
+ 'dataSource.driverClassName': 'org.h2.Driver',
+ 'dataSource.username' : 'sa',
+ 'dataSource.password' : '',
+ 'dataSource.dbCreate' : 'create-drop'
+ ]
+ if (setting != null) {
+ config['grails.gorm.unique.nulls'] = setting
+ }
+ manager.grailsConfig = config
+ manager.setup(this.class)
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: 'en', country: null).save(validate:
false, flush: true)
+ new UnqLanguageValue(language: 'en', country: null).save(validate:
false, flush: true)
+ }
+
+ expect: 'the database stays what H2 is, and the setting decides the
constraint without an option'
+ manager.session.datastore.databaseUniqueNulls == UniqueNulls.SKIP
+ manager.session.datastore.uniqueNulls == reading
+ UnqLanguage.withNewSession { UnqLanguage.withNewTransaction { !new
UnqLanguage(language: 'en', country: null).validate() } } == duplicate
+
+ and: 'a constraint that asks for value keeps it whatever the setting
says'
+ UnqLanguageValue.withNewSession { UnqLanguageValue.withNewTransaction
{ !new UnqLanguageValue(language: 'en', country: null).validate() } }
+
+ where:
+ setting || reading | duplicate
+ null || UniqueNulls.SKIP | false
+ 'database' || UniqueNulls.SKIP | false
+ 'skip' || UniqueNulls.SKIP | false
+ 'value' || UniqueNulls.VALUE | true
+ 'wildcard' || UniqueNulls.WILDCARD | true
+ }
+
+ @Unroll
+ void 'on #db an entirely null key is accepted by the database and by
validation'() {
+ given: 'the datastore on this database, with the schema created for it'
+ if (container != null && !container.isRunning()) {
+ container.start()
+ }
+ manager.destroy()
+ manager.grailsConfig = [
+ 'dataSource.url' : container?.jdbcUrl ?:
"jdbc:h2:mem:uniqueNullsAllNull${db};LOCK_TIMEOUT=10000".toString(),
+ 'dataSource.driverClassName': container?.driverClassName ?:
'org.h2.Driver',
+ 'dataSource.username' : container?.username ?: 'sa',
+ 'dataSource.password' : container?.password ?: '',
+ 'dataSource.dbCreate' : 'create-drop'
+ ]
+ manager.setup(this.class)
+ UnqEmail.withNewTransaction {
+ new UnqEmail(email: null).save(validate: false, flush: true)
+ }
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: null, country: null).save(validate:
false, flush: true)
+ }
+
+ when: 'a second entirely-null key is validated and saved'
+ boolean emailValid = UnqEmail.withNewSession {
+ UnqEmail.withNewTransaction {
+ new UnqEmail(email: null).validate()
+ }
+ }
+ boolean compositeValid = UnqLanguage.withNewSession {
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: null, country: null).validate()
+ }
+ }
+
+ boolean emailAccepted = false
+ try {
+ UnqEmail.withNewTransaction {
+ new UnqEmail(email: null).save(validate: false, flush: true)
+ }
+ emailAccepted = true
+ } catch (DataIntegrityViolationException ignored) {
+ }
+
+ boolean compositeAccepted = false
+ try {
+ UnqLanguage.withNewTransaction {
+ new UnqLanguage(language: null, country: null).save(validate:
false, flush: true)
+ }
+ compositeAccepted = true
+ } catch (DataIntegrityViolationException ignored) {
+ }
+
+ then: 'both rows pass validation and are accepted by the database'
Review Comment:
Added `manager.session.datastore.entirelyNullKeyExempt == exempt` assertion
to `UniqueNullsHibernate7Spec` with `false` for H2, PostgreSQL, MySQL, and
MariaDB, and `true` for Oracle.
##########
grails-doc/src/en/guide/upgrading/upgrading90x.adoc:
##########
@@ -1229,3 +1229,34 @@ See <<asyncPromises,Promises>> and <<events,Events>>.
`grails-async-rxjava3` carries a `null` value as the RxJava 1 factory and
every other promise factory do: `createPromise()` and `createPromise(Class)`
give a promise whose `get()` returns `null`, a task that returns `null`
resolves to `null`, and a `null` value keeps its place in the list that
`onComplete`, `onError` and `waitAll` deliver.
`grails-async-rxjava2` does not: there `createPromise()` and `onComplete` over
a list holding a bound promise of `null` fail with a `NullPointerException`,
and a task that returns `null` resolves to the `java.lang.Void` class.
+
+==== 20. A Null in a Unique Constraint's Composite Key Follows the Database
+
+A `unique` constraint that names other properties, such as `url unique:
'httpMethod'`, validates the constrained property together with the properties
of its group.
+Before Grails 9, a group property whose value was `null` was left out of the
query that looks for a conflicting row.
+The constraint then checked the constrained property alone, so `new
Language(language: 'en')` was rejected when a `Language` with `language: 'en'`
and `country: 'US'` existed, although the two rows differ in `country`.
+That was stricter than any reading of `null`: in SQL a `null` never conflicts,
and where `null` is a value it conflicts only with another `null`.
+
+In Grails 9 the constraint reads a `null` in its key, in the group and in the
constrained property, as the unique index of the database does, so that the
validation and the index agree:
+
+* `skip` on H2, PostgreSQL, MySQL, MariaDB, on SQL Server with Hibernate 7,
and on Db2 for IBM i and z/OS with Hibernate 7, whose index leaves a row with a
`null` out: `('en', null)` is never a duplicate.
+* `value` on Oracle, on Db2 for Linux, UNIX and Windows with Hibernate 7
(`exclude null keys` indexes partly null keys), on Db2, SQL Server and Sybase
with Hibernate 5 (a plain unique constraint compares `null` values), and on a
datastore with no such index such as MongoDB: a partly null key `('en', null)`
conflicts only with another `('en', null)`. On Oracle and on Db2 for Linux,
UNIX and Windows with Hibernate 7, a key that is `null` in every column is
omitted by the index, so `(null, null)` and an optional single property with
value `null` are accepted. See the per-database table in the <<unique,unique>>
constraint reference.
Review Comment:
Updated section 20 of `upgrading90x.adoc` to describe the rule for an
entirely null key under `database`: accepted on every `skip` database, on
Oracle and Db2 for LUW with Hibernate 7, and on MongoDB, Neo4j, and the
in-memory datastore; rejected on SQL Server, Sybase, and Db2 under Hibernate 5;
and rejected everywhere under an explicit `value`.
--
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]
