Skip to content

fail min facets on incomparable date and duration values - #132

Merged
pjfanning merged 1 commit into
apache:trunkfrom
aizu-m:min-facet-incomparable
Oct 5, 2026
Merged

pjfanning merged 1 commit into
apache:trunkfrom
aizu-m:min-facet-incomparable

Conversation

@aizu-m

@aizu-m aizu-m commented Oct 5, 2026

Copy link
Copy Markdown
Contributor
org.opentest4j.AssertionFailedError: minInclusive ==> expected: <false> but was: <true>
    at misc.checkin.IncomparableMinFacetValidateTest.incomparableDateTimeFailsEveryBound(IncomparableMinFacetValidateTest.java:81)

Found while reading the bound checks in JavaGDateHolderEx.validateValue. compareToGDate is documented to return 2 for an incomparable pair, and the four checks did not look like they agreed on what 2 means. Ran one bound against one value under each facet to see:

dateTime, bound 2000-01-01T12:00:00Z, value 2000-01-01T12:00:00
  minInclusive   validates
  minExclusive   validates
  maxInclusive   rejected
  maxExclusive   rejected

The value has no timezone and sits within 14 hours of the bound, so the order is indeterminate. The max checks test > 0 and >= 0. 2 satisfies both, so the value is reported. The min checks test < 0 and <= 0. 2 satisfies neither, so the value passes. It is not greater than or equal to the bound, so it should fail the min facets too.

JavaGDurationHolderEx.validateValue has the same shape and gives the same result: P1M against a bound of P30D validates under both min facets and is rejected under both max facets.

The schema compiler already takes this view. StscSimpleTypeResolver refuses a derived bound when comparison == 2.

The fix reports 2 in the two min checks of each holder. Only values whose order against the bound is indeterminate change, and the max facets already reject those. The float, double, decimal and integer holders compare with a total order, so they are untouched.

Regression test added in IncomparableMinFacetValidateTest. On trunk the two incomparable cases fail as above, with the fix all four pass, and ./gradlew test is green.

AI tooling was used to help prepare this change.

compareToGDate and compareToGDuration return 2 for an incomparable pair. The max facet checks already report that as invalid; the min checks let it through. Report it there too.

@pjfanning pjfanning left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@pjfanning
pjfanning merged commit 3a27438 into apache:trunk Oct 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants