-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Description
Summary
The semantics of Specification.unrestricted()
are inconsistent across different logical operator methods (or
, and
, not
, etc.), leading to confusion and potentially incorrect assumptions when composing specifications.
Background
The documentation describes Specification.unrestricted()
as:
Simple static factory method to create a specification matching all objects.
This suggests a mental model akin to:
return (root, query, builder) -> builder.conjunction(); // i.e., always true
However, the actual implementation is:
return (root, query, builder) -> null;
And per the specification contract:
Specifications returning
null
are considered to not contribute to the overall predicate and their result is not considered in the final predicate.
This leads to two competing interpretations:
- Interpretation A:
unrestricted()
behaves liketrue
- Interpretation B:
unrestricted()
behaves like SQLNULL
or theUnknown
state in three-valued logic.
Observed Behaviour
In practice, unrestricted()
behaves like NULL
in most logical operators:
Predicate predicate = ... ;
Specification other = (r, q, cb) -> predicate;
assertThat(unrestricted().or(other).toPredicate(root, query, builder))
.isSameAs(predicate);
assertThat(unrestricted().and(other).toPredicate(root, query, builder))
.isSameAs(predicate);
assertThat(allOf(unrestricted(), other).toPredicate(root, query, builder))
.isSameAs(predicate);
assertThat(anyOf(unrestricted(), other).toPredicate(root, query, builder))
.isSameAs(predicate);
This implies that unrestricted()
is ignored in composition, consistent with the null
return value.
However, Specification.not(..)
behaves differently:
assertThat(not(unrestricted()).toPredicate(root, query, builder)).isNull(); // fails
Instead of returning null
, it returns builder.disjunction()
(i.e., false
), based on reasoning from #3849 (emphasis mine):
In the current semantics, if specification.toPredicate() returns null, it means no condition—which logically translates to an Expression true.
Thus, in the not() method, we can return criteriaBuilder.disjunction() to represent false, because:
not(true) → false
This implies that unrestricted()
is treated as true
in the context of not()
, breaking consistency.
Consequences
This inconsistency breaks logical equivalences. For example:
Specification a = Specification.unrestricted();
Specification b = ...;
Specification c = not(a.or(b));
Specification d = not(a).and(not(b));
Logically, c
and d
should be equivalent (!(a || b) == !a && !b
), but:
c
becomesnot(b)
d
becomesfalse
Proposed Solutions
Option 1: Document the inconsistency
Clarify in the documentation that Specification.unrestricted()
behaves like NULL
in most logical operations, but is treated as true
in not()
. This avoids breaking changes but requires users to be cautious when composing specifications.
Option 2: Make not(unrestricted())
return unrestricted()
This would make unrestricted()
consistently behave like NULL
, preserving logical equivalences. However, it breaks existing behaviour where not(unrestricted())
is expected to match no results.
Option 3: Make or()
and anyOf()
treat unrestricted()
as true
This aligns with the idea that unrestricted()
matches all objects. It would make unrestricted().or(other)
behave like true || other
, but introduces broader breaking changes.
Request for Feedback
Would the maintainers prefer Option 1 (document), Option 2 (change not()
), Option 3 (change or()
/anyOf()
), or another approach?
Happy to submit a PR based on the preferred direction.