borinquenkid commented on code in PR #16583:
URL: https://github.com/apache/grails-core/pull/16583#discussion_r4360884726
##########
grails-doc/src/en/guide/upgrading/upgrading90x.adoc:
##########
@@ -305,3 +305,22 @@ It applies nothing else. Null properties are written
whatever `@JsonInclude` or
`spring.jackson.default-property-inclusion` say, properties are written in the
converter's order rather than by
`@JsonPropertyOrder` or sorted alphabetically, and `@JsonView`, `@JsonFilter`,
`@JsonAnyGetter`, `@JsonIgnoreType`,
`@JsonFormat` and `@JsonSerialize` on a property, and mix-ins, other than the
property names they give, do not apply.
+
+==== 5. A Null Value in the Group of a Unique Constraint Is Compared as a Value
+
+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`.
+
+In Grails 9 a `null` group value is compared as a value: only a row whose
value of that group property is also `null` conflicts.
+With `language unique: 'country'`, `('en', null)` no longer conflicts with
`('en', 'US')`, and a second `('en', null)` still does.
+The same holds for a nullable association in the group, so a folder with `name
unique: 'parent'` may share its name with a folder under a parent, while two
top level folders with the same name still conflict.
+
+A group with several properties compares every one of them, so `('A', null,
'web')` conflicts only with a row that has code `A`, no region and channel
`web`.
+
+Applications that relied on the old behaviour are affected: a save that was
rejected because a `null` group value matched every row of the constrained
property now succeeds when the rows differ in that group property.
+The old behaviour was a defect and has no replacement.
+To keep a rule such as "the name is unique whatever the value of `parent`",
declare the constraint on the one property, `name unique: true`.
+If the database must also reject the second `('en', null)` row, remember that
most databases do not treat `null` values of a unique index as equal; the
validation does, the index does not.
Review Comment:
Replaced "most databases" with the table, and made it the default's rule. I
checked it rather than took it from memory: on H2, PostgreSQL, MySQL, MariaDB
and Oracle containers, `UniqueNullsHibernate7Spec` inserts `('en', null)` twice
and compares the database's answer with the validation's. Oracle rejects it (it
compares the non-null columns of a partly null key), the other four accept it.
One correction to the list in the review: SQL Server and DB2 read `skip`, not
`value`, because Hibernate creates the index over nullable columns as a
filtered unique index (`where col is not null`) and DB2's as `exclude null
keys`, so rows with a null are left out. That is from the DDL Hibernate
renders; I have no SQL Server or DB2 container. So the default is `skip`
everywhere except Oracle, where it is `value`.
##########
grails-doc/src/en/ref/Constraints/unique.adoc:
##########
@@ -64,6 +64,8 @@ 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.
+A `null` value of a property in the group is compared as a value: it conflicts
only with a row whose value of that property is also `null`. With `group
unique: 'department'`, a group without a `department` may have the same name as
a group of a department, but two groups with that name and no `department`
conflict. This is the same for a group that has several properties and for an
association in the group, such as a `parent` that is `null` for the top level
nodes of a tree.
Review Comment:
Done in 70a0b0220a: `unique.adoc` describes the four readings in a table,
the reading each database gets under `database`, the map form, the setting, and
says the constrained property follows the same reading. The GORM configuration
page lists the setting.
##########
grails-datamapping-tck/src/main/groovy/org/apache/grails/data/testing/tck/tests/UniqueConstraintNullGroupValueSpec.groovy:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.PendingFeatureIf
+
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.apache.grails.data.testing.tck.domains.UniqueNullFolder
+import org.apache.grails.data.testing.tck.domains.UniqueNullLanguage
+import org.apache.grails.data.testing.tck.domains.UniqueNullMulti
+import org.apache.grails.data.testing.tck.domains.UniqueNullRoute
+
+/**
+ * A null value of a property in the group of a {@code unique} constraint is
compared as a value:
+ * it only conflicts with the rows whose value of that property is also null.
+ *
+ * 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 UniqueConstraintNullGroupValueSpec extends GrailsDataTckSpec {
Review Comment:
Done in 70a0b0220a. `UniqueConstraintNullsSpec` (TCK) runs the same four
rows and six probes against a nullable `(language, country)` key per reading,
plus the database-following one checked against what the datastore says its
database does. `UniqueNullsSpec` (in-memory datastore) adds the `Folder`-shaped
association key under `wildcard`, `skip` and `value`, and the setting values.
The constrained-property-null probes, `(null, 'US')` and `(null, null)`, are
rows in both, so that column's rule is pinned on purpose.
`UniqueConstraintNullGroupValueSpec` keeps its shapes and names `value`
explicitly.
--
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]
