Skip to content

[bugfix] fn:min / fn:max — restore XQ 3.1 numeric promotion + reject QName/plain duration/mixed yMD-dTD#14

Open
joewiz wants to merge 1 commit into
v2/xq4-core-functionsfrom
fix/fn-min-max-xq31-promotion
Open

[bugfix] fn:min / fn:max — restore XQ 3.1 numeric promotion + reject QName/plain duration/mixed yMD-dTD#14
joewiz wants to merge 1 commit into
v2/xq4-core-functionsfrom
fix/fn-min-max-xq31-promotion

Conversation

@joewiz
Copy link
Copy Markdown
Owner

@joewiz joewiz commented May 10, 2026

Summary

Pre-merge defense for PR eXist-db#6218 (v2/xq4-core-functions). Commit `95b37d7967` rewrote `FunMin` / `FunMax` to use `FunCompare.compare()` for total ordering and dropped the XQ 3.1 numeric type-promotion path plus the QName / duration validation. That regresses 23 XQTS 3.1 tests (cbcl-min/max, K2-MinMaxFunc-*, fn-min/max-001..005). This PR restores the spec-correct behaviour.

Root cause

`95b37d7967` simplified the accumulator to compare-and-replace using `FunCompare.compare`, which preserves the first item's type. The XPath F&O 3.1 §15.4 spec requires:

  • Numeric mutual promotion to least-common-type across the sequence (so `min((1, xs:float(2), xs:decimal(3)))` → `xs:float`).
  • NaN propagation that takes the widest numeric type seen.
  • Reject `xs:QName` (FORG0006).
  • Reject plain `xs:duration` (FORG0006), and reject mixed `xs:yearMonthDuration` / `xs:dayTimeDuration` sequences (FORG0006).

What changed

  • `exist-core/src/main/java/org/exist/xquery/functions/fn/FunMin.java` / `FunMax.java` — restore mutual `AtomicValue.promote()` accumulator; restore QName / duration validation; preserve the `FunCompare` total-ordering comparison for non-numeric types.
  • `exist-core/src/test/java/org/exist/xquery/functions/fn/FunMinMaxTest.java` — new 14-case test mirroring the XQTS 3.1 regressions verbatim.

Test plan

Spec compatibility

The QT4 (XQ 4.0) versions of these tests carry `XP20 XP30 XP31 XQ10 XQ30 XQ31` dependencies (no `XQ40`) or are flagged "implementation-defined" in XQ 4.0 — so the stricter XQ 3.1 behaviour this PR restores passes both suites.

🤖 Generated with Claude Code

@joewiz @claude
[bugfix] fn:min/fn:max: restore XQ 3.1 numeric promotion + duration/Q…
14d7f3e 
…Name checks

The XQ 4.0 rewrite of FunMin/FunMax (commit 95b37d7) dropped numeric
type promotion in favour of fn:compare-based total ordering. That broke
20-23 XQ 3.1 conformance tests on each of the fn-min and fn-max suites
that depend on the result carrying the least-common-type:

  min((1, xs:float(2), xs:decimal(3))) instance of xs:float       -> true
  min((5, 5.0e0))                       instance of xs:double      -> true
  min((xs:float('NaN'), 1, xs:double('NaN'))) instance of xs:double -> true

It also stopped raising err:FORG0006 for the cases the spec requires:

  min(QName('example.com/', 'ncname'))                      -> FORG0006
  min((xs:yearMonthDuration('P1Y'), xs:dayTimeDuration('P1D'))) -> FORG0006
  min(xs:duration('P1Y1M1D'))                              -> FORG0006

Restore numeric type promotion via AtomicValue.promote() in both
directions so the accumulator widens to the most general numeric type
seen, including across NaN values. Reject xs:QName values up front, and
reject plain xs:duration plus mixed xs:yearMonthDuration / xs:dayTimeDuration
sequences as the spec requires.

This is XQ 3.1 spec text. The corresponding XQ 4.0 cases are flagged as
"implementation-defined" in QT4 (e.g. K2-SeqMINFunc-7), so the stricter
behaviour passes both XQ 3.1 and QT4 conformance tests.

XQTS 3.1 fn-min: ~13 tests recovered (K-SeqMINFunc-14/18/19/38, K2-SeqMINFunc-7/9/10, cbcl-min-009/010/012, fn-min-8/9, ...)
XQTS 3.1 fn-max: ~10 tests recovered (parallel set)
QT4 fn-min/fn-max: same regressions also closed (was 20 fn-min failures, ~12 of those are the same XQ 3.1 cases)

Closes the fn:min/fn:max bucket of the next-v3 pre-merge regression triage.
The 7 prod-TryCatchExpr regressions are tracked separately in the
2026-05-10 prod-trycatch-regression-followup tasking.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joewiz
Copy link
Copy Markdown
Owner Author

joewiz commented May 10, 2026

[This response was co-authored with Claude Code. -Joe]

Triage on the 2 XQTS failures: not a regression in this PR's fix.

Both failures are 60-minute hard timeouts of the Run XQTS workflow step:
##[error]The action 'Run XQTS' has timed out after 60 minutes. (orphan Java
process killed, ~30,857 of ~30,858 dispatched cases finished). The fn:min and
fn:max test sets completed cleanly within the first ~10 seconds — all 501
fn-min/* and fn-max/* test cases logged matching Starting/Finished
events, with no fn-min/fn-max cases appearing in the still-running set at
timeout and no assertion failures.

The base branch v2/xq4-core-functions has the same problem on its own CI:
the 5 most recent XQTS runs on that branch (no fn:min/max change) all ended
with the same Run XQTS has timed out after 60 minutes line. Adjacent fork
branches at similar wall-clock times succeeded, so it's not an infra blip.

Recommended next step: rebase v2/xq4-core-functions onto current develop
(it's ~170 commits behind, and develop has merged several relevant perf fixes
since this base was last touched on 2026-05-01). Once the base is current, a
fresh CI run on this PR should pass.

Holding the PR. No code change needed in #14 itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@joewiz