jdaugherty commented on code in PR #16583: URL: https://github.com/apache/grails-core/pull/16583#discussion_r4412994998
########## grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/UniqueConstraintNullsSpec.groovy: ########## @@ -0,0 +1,104 @@ +/* + * 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 org.apache.grails.data.testing.tck.tests + +import spock.lang.IgnoreIf +import spock.lang.Issue +import spock.lang.Unroll + +import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec +import org.apache.grails.data.testing.tck.domains.UniqueNullKeyDatabase +import org.apache.grails.data.testing.tck.domains.UniqueNullKeySkip +import org.apache.grails.data.testing.tck.domains.UniqueNullKeyValue +import org.apache.grails.data.testing.tck.domains.UniqueNullKeyWildcard +import org.grails.datastore.mapping.config.UniqueNulls + +/** + * How the {@code unique} constraint reads a {@code null} in a composite key, per reading: the same rows and the same + * probes against a language whose constraint asks for each reading. A reading that follows the database is checked + * against the reading the datastore says its database has. + * + * The simple in-memory datastore of the TCK does not register the {@code unique} constraint, so this specification + * does not run there; {@code UniqueConstraintSpec} of {@code grails-datamapping-core-test} covers it. + */ +@IgnoreIf({ Boolean.getBoolean('simple.gorm.suite') }) +@Issue('https://github.com/apache/grails-core/issues/14503') +class UniqueConstraintNullsSpec extends GrailsDataTckSpec { + + void setupSpec() { + manager.registerDomainClasses(UniqueNullKeyWildcard, UniqueNullKeySkip, UniqueNullKeyValue, UniqueNullKeyDatabase) + } + + @Unroll + void 'with the #reading reading, the probe (#language, #country) conflicts: #conflicts'() { + given: 'the rows a key may have, saved without validation since some of them conflict under some readings' + [['en', 'US'], ['en', null], [null, 'US'], [null, null]].each { String l, String c -> + domain.newInstance(language: l, country: c).save(validate: false, flush: true) + } + + expect: + !domain.newInstance(language: language, country: country).validate() == conflicts + + where: + reading | domain | language | country || conflicts + 'wildcard' | UniqueNullKeyWildcard | 'en' | 'US' || true + 'wildcard' | UniqueNullKeyWildcard | 'en' | null || true + 'wildcard' | UniqueNullKeyWildcard | 'en' | 'FR' || false + 'wildcard' | UniqueNullKeyWildcard | null | 'US' || false + 'wildcard' | UniqueNullKeyWildcard | null | null || false + 'wildcard' | UniqueNullKeyWildcard | 'fr' | null || false + 'skip' | UniqueNullKeySkip | 'en' | 'US' || true + 'skip' | UniqueNullKeySkip | 'en' | null || false + 'skip' | UniqueNullKeySkip | 'en' | 'FR' || false + 'skip' | UniqueNullKeySkip | null | 'US' || false + 'skip' | UniqueNullKeySkip | null | null || false + 'skip' | UniqueNullKeySkip | 'fr' | null || false + 'value' | UniqueNullKeyValue | 'en' | 'US' || true + 'value' | UniqueNullKeyValue | 'en' | null || true + 'value' | UniqueNullKeyValue | 'en' | 'FR' || false + 'value' | UniqueNullKeyValue | null | 'US' || true + 'value' | UniqueNullKeyValue | null | null || true + 'value' | UniqueNullKeyValue | 'fr' | null || false + } + + @Unroll + void 'a constraint without a reading follows the database for the probe (#language, #country)'() { + given: + UniqueNulls databaseReading = manager.session.datastore.databaseUniqueNulls + [['en', 'US'], ['en', null], [null, 'US'], [null, null]].each { String l, String c -> + UniqueNullKeyDatabase.newInstance(language: l, country: c).save(validate: false, flush: true) + } + + expect: 'the datastore answers SKIP or VALUE, and the constraint reads the same' + databaseReading in [UniqueNulls.SKIP, UniqueNulls.VALUE] + manager.session.datastore.uniqueNulls == databaseReading + !new UniqueNullKeyDatabase(language: language, country: country).validate() == + (databaseReading == UniqueNulls.VALUE ? valueConflicts : skipConflicts) Review Comment: This row fails on MongoDB and Neo4j at this head (every Mongo and Neo4j job of the run, the coverage job and the Groovy canary, one failure each: the `(null, null)` probe). `MongoDatastore` and `Neo4jDatastore` answer `value` from the `Datastore` default of `getDatabaseUniqueNulls()` and `true` from the new default of `isEntirelyNullKeyExempt()`, so `UniqueConstraint` returns before the lookup and the second `(null, null)` validates, while `valueConflicts` says it conflicts. The default is the right answer for a datastore without an index of its own: nothing there rejects the row, and keeping a second `null` valid for `email nullable: true, unique: true` is what the round-3 comment asked for. So the spec should follow the flag the way it follows `databaseUniqueNulls`: read `manager.session.datastore.entirelyNullKeyExempt` in `given:` and expect `skipConflicts` for the entirely null probe when it is set, `valueConflicts` otherwise. If you would rather have MongoDB and Neo4j reject it, make them answer `false` and keep the row; either way the spec, the default and the reference must agree. ########## grails-doc/src/en/ref/Constraints/unique.adoc: ########## @@ -64,6 +64,83 @@ username(unique: ['group', 'department']) In this example `username` must be unique in `group` and `department`. There might be same logins in different groups or different departments. +==== Null values in a composite key + +A property of the key can be `null`: a `country` that is optional in `language unique: 'country'`, a `parent` that is `null` for the top level nodes of a tree, or the constrained property itself when it is `nullable`. Whether such a row can be a duplicate of another is a question databases answer in two ways for their unique index, so the constraint has four readings of a `null`, selected per constraint or for the whole application: + +[cols="1,4"] +|=== +|Reading |A `null` in the key + +|`database` (default) +|Is read the way the unique index of the database does: `skip` where the index treats `null` values as distinct, `value` where it compares them. The validation and the index then agree. + +|`skip` +|Is never a duplicate, as in a SQL unique predicate: `('en', null)` twice is valid, and the lookup is not made. + +|`value` +|Is a value that matches only another `null`: `('en', null)` conflicts with another `('en', null)` and with no row that has a country. This holds for the constrained property as well. + +|`wildcard` +|In the group, leaves its property out of the lookup, so it matches every row: `('en', null)` conflicts with `('en', 'US')`. A `null` constrained property is not checked. +|=== + +With the rows `('en', 'US')`, `('en', null)`, `(null, 'US')` and `(null, null)`, a new `('en', null)` is a duplicate under `value` and `wildcard` and is valid under `skip`; a new `(null, 'US')` is a duplicate only under `value`, where it matches the stored `(null, 'US')`. + +The unique index Hibernate creates for the group treats `null` values as follows, which is what `database` reads. The two Hibernate versions render the index differently for SQL Server and Db2, so the table says which version each row describes: + +[cols="2,1,1,3"] +|=== +|Database |Hibernate |Reading |Why + +|H2, PostgreSQL, MySQL, MariaDB +|5 and 7 +|`skip` +|`null` values are distinct in a unique index. + +|SQL Server +|7 +|`skip` +|Hibernate 7 creates the index over nullable columns as a filtered unique index that leaves the rows with a `null` out. + +|Db2 for IBM i, Db2 for z/OS +|7 +|`skip` +|Hibernate 7 creates the index as `create unique where not null index`, under which `null` values are not equal to each other. + +|SQL Server, Sybase +|5 +|`value` +|Hibernate 5 has no filtered-index delegate, so these dialects get a plain unique constraint, which compares `null` values: `('en', null)` twice is a violation. + +|Db2 (all platforms) +|5 +|`value` +|Hibernate 5 creates a plain unique index over nullable columns (`DB2UniqueDelegate`), which compares `null` values. + +|Db2 for Linux, UNIX and Windows +|7 +|`value` +|Hibernate 7 creates the index with `exclude null keys`, which leaves out only a key that is `null` in every column; a key with a `null` in some columns is indexed and compared. + +|Oracle +|5 and 7 +|`value` +|A row with a `null` in some of the key columns is indexed, and its other columns are compared, so `('en', null)` twice is a violation. +|=== + +A datastore that keeps no such index, such as MongoDB, reads `database` as `value`. Review Comment: The upgrade note sends the reader here for what happens to a key that is `null` in every column, and this page does not say. Please add the rule next to the table: under `database` an entirely null key is accepted everywhere except SQL Server, Sybase and Db2 on Hibernate 5, where the plain unique constraint compares it; an explicit `value`, on the constraint or in `grails.gorm.unique.nulls`, rejects it on every datastore. This sentence is the place for the datastores without an index: MongoDB, Neo4j and the in-memory datastore of unit tests read `value` for a partly null key and accept an entirely null one. ########## grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/dialect/UniqueNullsPerDialect.java: ########## @@ -0,0 +1,84 @@ +/* + * 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 org.grails.orm.hibernate.dialect; + +import org.hibernate.dialect.DB2Dialect; +import org.hibernate.dialect.DB2iDialect; +import org.hibernate.dialect.DB2zDialect; +import org.hibernate.dialect.Dialect; +import org.hibernate.dialect.OracleDialect; + +import org.grails.datastore.mapping.config.UniqueNulls; + +/** + * How the unique index Hibernate creates for a {@code unique} constraint with a group treats a {@code null} in the key, + * on each database, so that the validation can read {@code null}s the way the index will. + * <ul> + * <li>{@link #ORACLE} and {@link #DB2}: Oracle indexes a row that has a {@code null} in some of the key columns and + * compares its other columns, so {@code ('en', null)} twice is a violation, and only a key that is {@code null} in + * every column is left out. Db2 for Linux, UNIX and Windows comes to the same: Hibernate creates the index with + * {@code exclude null keys}. A {@code null} is a value: {@link UniqueNulls#VALUE}.</li> + * <li>{@link #DB2_IBM_I}, {@link #DB2_ZOS} and {@link #DEFAULT}: H2, PostgreSQL, MySQL and MariaDB treat {@code null}s + * as distinct, so such a row never conflicts. SQL Server compares them, but Hibernate renders the index for a nullable + * key as a filtered unique index that leaves the rows with a {@code null} out, which comes to the same, and on Db2 for + * IBM i and z/OS as {@code create unique where not null index}, under which {@code null}s are not equal. + * {@link UniqueNulls#SKIP}.</li> + * </ul> + * + * @since 9.0 + */ +public enum UniqueNullsPerDialect implements DialectKeyed<UniqueNullsPerDialect> { + + ORACLE(UniqueNulls.VALUE, OracleDialect.class), Review Comment: Verified at da18b6b6ab: `ORACLE` and `DB2` (Linux, UNIX and Windows) carry the exemption on both enums, both datastores expose it, and `UniqueConstraint` returns before the lookup for a key that is `null` in every column under `database`. The Hibernate 7 jobs ran the new all-null feature on H2, PostgreSQL, MySQL, MariaDB and 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: The entirely null key is accepted under `database` on more than these two: on every `skip` database, where the constrained property is not checked, and on MongoDB, Neo4j and the in-memory datastore of unit tests, which have no index to disagree with. It is rejected only on SQL Server, Sybase and Db2 under Hibernate 5, and everywhere under an explicit `value`. Please say that here, since the unit-test datastore is where most people meet it, or point to the reference once it carries the rule (see the comment on `unique.adoc`). ########## grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy: ########## @@ -100,6 +153,12 @@ class UniqueConstraint extends AbstractConstraint { throw new IllegalStateException("Cannot validate object [$target]. It is not a persistent entity") } + UniqueNulls nullsReading = resolveNulls(GormRegistry.findDatastore(constraintClass)) + if (propertyValue == null && nullsReading != UniqueNulls.VALUE) { Review Comment: Verified: `NullsSingleProperty` on the in-memory datastore and `UnqEmail` on the five Hibernate 7 databases accept a second `null` under `database`, and `NullsSinglePropertyValue` rejects it under `value`. The same default is what fails the TCK row on MongoDB and Neo4j; see the comment on `UniqueConstraintNullsSpec`. ########## 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 and DB2, and on SQL Server with Hibernate 7, whose index leaves a row with a `null` out: `('en', null)` is never a duplicate. Review Comment: Verified: Db2 is split between the two bullets as in the table. ########## grails-data-neo4j/core/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/Neo4jQuery.groovy: ########## @@ -335,6 +335,11 @@ class Neo4jQuery extends Query { @Override @CompileStatic CypherExpression handle(GraphPersistentEntity entity, Query.IsNull criterion, CypherBuilder builder, String prefix) { + PersistentProperty prop = entity.getPropertyByName(criterion.property) Review Comment: Confirmed by reading at da18b6b6ab: `buildBaseQuery` binds the relationship as `r` and passes it as the prefix, and `RelationshipPersistentEntity.formatAssociationPatternFromExisting` renders `(r)-[]->(:Movie)` from it, which Neo4j refuses as a type mismatch on `r`. Before this change `RelationshipPersistentEntity.formatProperty` returned the bare endpoint name, so the criterion was `from IS NULL`, valid Cypher. A guard before the pattern does it: when the entity is a `RelationshipPersistentEntity` and the property is `from` or `to`, fall through to `formatProperty`, as the `Equals` handler already treats `isRelationshipAssociation` apart. ########## 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: The default is `true` for a datastore without a unique index of its own as well (`MongoDatastore`, `Neo4jDatastore` and `SimpleMapDatastore` inherit it), and there it is not an index omitting the key but the absence of one. Please say so in the javadoc, since this is what the `DataTest` datastore of a unit test answers. ########## 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: Non-blocking: on H2, PostgreSQL, MySQL and MariaDB this passes through `skip`, so the flag is exercised on Oracle alone. A `manager.session.datastore.entirelyNullKeyExempt == exempt` line with a column (`false` for the four, `true` for Oracle) would pin it against the database the way the first feature pins `databaseUniqueNulls`. -- 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]
