borinquenkid commented on code in PR #16583:
URL: https://github.com/apache/grails-core/pull/16583#discussion_r4360882388
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy:
##########
@@ -157,6 +157,9 @@ class UniqueConstraint extends AbstractConstraint {
}
}
eq(propName, value)
+ } else {
+ // null is compared as a value: only a row whose
value is also null conflicts
+ isNull(propName)
Review Comment:
Done in 70a0b0220a. `UniqueConstraint` takes the map form you proposed,
`unique: [group: ..., nulls: ...]`, next to the `Iterable` and `CharSequence`
forms, with the three readings you listed as `wildcard`, `skip` and `value`,
and a fourth, `database`, which is the default and follows the unique index of
the database. `grails.gorm.unique.nulls` sets the reading for every constraint
without one. Unknown map keys or an unknown reading fail when the constraint is
created. Hibernate's mapping builders and `Property` read the map form as its
group, otherwise it created a single-column unique index.
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy:
##########
@@ -157,6 +157,9 @@ class UniqueConstraint extends AbstractConstraint {
}
}
eq(propName, value)
+ } else {
Review Comment:
Done in 70a0b0220a: the toggle governs both columns. Under `skip` a null
constrained value is not checked, under `value` it is compared with `isNull`,
and under `wildcard` it is not checked, since the constrained property has no
wildcard reading. `skipNullValues()` now returns false so the constraint
decides. The constrained-property-null probes are rows in
`UniqueConstraintNullsSpec` and `UniqueNullsSpec`.
##########
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
Review Comment:
Renumbered when 9.0.x was merged: the section is 17 at this head. The
section is rewritten for the toggle, so the number is the only thing that stays.
##########
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.
Review Comment:
Rewritten in 70a0b0220a: the note names the four readings, the default, and
the line that keeps the previous behavior (`nulls: 'wildcard'`, or
`grails.gorm.unique.nulls: wildcard` for the application). It also says `name
unique: true` is not a replacement, since it makes the name unique across every
parent.
--
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]
