jdaugherty commented on code in PR #16583:
URL: https://github.com/apache/grails-core/pull/16583#discussion_r4392719339
##########
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:
This line affects constraints without a group as well. On 9.0.x,
`AbstractConstraint.validate` returns before `processValidate` when the value
is `null`, so `email nullable: true, unique: true` never checked a `null`. With
`skipNullValues()` returning `false`, a `null` now reaches the lookup whenever
the reading is `value`. Under the default, `value` is the reading of Oracle, of
SQL Server, Sybase and Db2 on Hibernate 5, of MongoDB and Neo4j, and of the
in-memory datastore. `DataTest` registers `UniqueConstraint` on that in-memory
datastore (`DataTestSetupSpecInterceptor`), so an application unit test that
saves two instances without an email now fails validation. I confirmed that
with a throwaway spec on `SimpleMapDatastore`: a second `email: null` fails
`validate()` with `unique`.
On Oracle this is the all-null case from the comment on the Hibernate 7
`UniqueNullsPerDialect`. Elsewhere the validator agrees with an index that
admits a single `null`, but every application with an optional unique property
still sees the change, and section 20 talks only about the group. If the
all-null key goes unchecked by default, a constraint without a group behaves as
it did before 9 wherever the index leaves that key out. In that case, the
change left for section 20 is the list of datastores where a single `null` is
now enforced. Either way, please add a `UniqueNullsSpec` row for a nullable
property with no group.
##########
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:
The javadoc above says that Oracle, and Db2 for LUW through `exclude null
keys`, leave out a key that is `null` in every column. `value` still looks that
key up with `isNull` on every column, so `database` rejects rows that these two
databases accept. The Hibernate 5 `ORACLE` constant has the same gap.
I checked this with a throwaway copy of `UniqueNullsHibernate7Spec` on the
same containers (not pushed):
| Database | Key | Database accepts the second row | `validate()` |
|---|---|---|---|
| Oracle | `email nullable: true, unique: true`, two `null` emails | yes
(`email varchar2(255 char) unique`) | rejects |
| Oracle | `language unique: 'country'`, two `(null, null)` | yes (`unique
(country, language)`) | rejects |
| H2, PostgreSQL | both keys | yes | accepts |
Please make the `database` reading for these dialects skip the lookup when
the constrained value and every group value are `null`, and add those two rows
to `UniqueNullsHibernate7Spec` so the Oracle container pins them. Hibernate 5
on SQL Server, Sybase and Db2 does compare such a key, because its plain unique
constraint or index admits a single `null`, so this has to be a property of the
dialect constant rather than a change to `value` itself.
##########
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:
Since d7079ef36f, Db2 reads `value` on Hibernate 5 and on Db2 for LUW with
Hibernate 7. It reads `skip` only for Db2 for IBM i and z/OS on Hibernate 7,
which matches the table in `unique.adoc`. This bullet still lists DB2 under
`skip`, and the `value` bullet below leaves it out. Please either split Db2 the
same way here or point to the table in the constraints reference, so the two
lists cannot drift apart again.
--
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]
