fix(core)!: require exactly one ordering expression for RANGE window bounds

[anasik]

• Spec v0.102.0 states this rule at site/docs/expressions/window_functions.md:34, and in algebra.proto at WindowRelFunction.bounds_type (:457) and Expression.WindowFunction.sorts (:1401)
• Requires exactly one, non-CLUSTERED ordering expression for a RANGE bound with a Preceding/Following side
• Fixed 4 pre-existing test fixtures that were building this invalid shape
• Adds a sorts-carrying overload to SubstraitBuilder.windowFn, since the existing one left every RANGE frame built through the DSL with no way to supply the ordering this check now requires

BREAKING CHANGE: a ConsistentPartitionWindow or WindowFunctionInvocation with a RANGE bound's Preceding or Following side now requires exactly one, non-CLUSTERED ordering expression. A plan that
previously built or parsed with zero, multiple, or a CLUSTERED ordering expression in that position now throws IllegalArgumentException.

[alexandrefimov]

Checked the three rules against algebra.proto at v0.102.0, the version the catalog pins: the bounds_type comment on WindowRelFunction carries both halves for the relation, and for the expression form the "exactly one ordering expression" half lives on the BOUNDS_TYPE_RANGE enum, which both kinds share. So both checks land where the spec puts them.

Three things I would add, none of them blocking.

The expression-side check has no test behind it. Neutering checkRangeOrdering and running :core:test fails exactly three of 743 tests — the three new ones, all on ConsistentPartitionWindow. Expression.WindowFunctionInvocation.check() gained the same call, and the fixture in windowFunctionInvocationRoundtripWithNonLiteralOffsetExpr needed its sort precisely because of it, but nothing pins the rejection there. The two checks sit in different classes and can regress independently, so one mirror of rangePrecedingWithTwoOrderingExpressionsIsRejected on the invocation would cover it.

On the issue's open question about CLUSTERED reachability: isthmus cannot emit it on a window — WindowFunctionConverter maps a window's collations through SortFieldConverter.asSortDirection, which throws for any direction but ASCENDING and DESCENDING. The CLUSTERED mapping isthmus does have, SubstraitRelVisitor.asSortDirection, is on the SortRel path. The one producer in the repo that builds a ConsistentPartitionWindow from user input is Spark's visitWindow, which takes its sorts straight from window.orderSpec. Worth a line in the description naming what this can newly reject, since the BREAKING CHANGE note reads as if any producer might be affected.

The third rule in that paragraph cannot be checked at all: Expression.SortField models only expr() and direction(), so a custom comparison function has no POJO representation to reject. The issue says so; the description does not, and the paragraph reads as fully enforced without it.

[nielspardon]

Add a sorts-carrying overload to SubstraitBuilder.windowFn — it never sets sort(), so after this change every RANGE frame with a Preceding/Following bound built through the DSL throws with no way for the caller to supply the ordering the check now demands (sb.windowFn(FUNCTIONS_ARITHMETIC, "lead:any", R.I64, INITIAL_TO_RESULT, ALL, RANGE, WindowBound.Preceding.of(5), WindowBound.CURRENT_ROW) → ...requires exactly one ordering expression, but found 0); ExpressionCreator.windowFunction already takes a sort list. Also retitle to fix(core)!: — the squash-merge message comes from the PR title, so without the ! the CHANGELOG entry carries no breaking marker.

Separately from the comparison-function clause already raised above: #1198's offset/ordering type-compatibility rule (add(T, D) -> T) is untouched, and that issue's own open question asked for a decision on it. Is Closes #1198 intended here, or should that rule get a follow-up issue first?

[anasik]

@nielspardon, I think it should get a follow-up issue.

[alexandrefimov]

Re-measured on 5127ae4: with the checkRangeOrdering call dropped from WindowFunctionInvocation.check() and the relation's left in place, :core:test (743) and :isthmus:test (1239) are both green, so nothing pins the expression side. This mirror fails without that call and passes with it, and needs no import ConsistentPartitionWindowRelRoundtripTest doesn't already have:

@Test
void rangePrecedingWithTwoOrderingExpressionsOnAnInvocationIsRejected() {
  SimpleExtension.WindowFunctionVariant declaration =
      extensions.getWindowFunction(
          SimpleExtension.FunctionAnchor.of(
              DefaultExtensionCatalog.FUNCTIONS_ARITHMETIC, "lead:any"));
  Expression.SortField sort =
      Expression.SortField.builder()
          .expr(sb.i64(1))
          .direction(Expression.SortDirection.ASC_NULLS_FIRST)
          .build();

  assertThrows(
      IllegalArgumentException.class,
      () ->
          Expression.WindowFunctionInvocation.builder()
              .declaration(declaration)
              .arguments(Collections.emptyList())
              .partitionBy(Collections.emptyList())
              .sort(Arrays.asList(sort, sort))
              .outputType(R.I64)
              .aggregationPhase(Expression.AggregationPhase.INITIAL_TO_RESULT)
              .invocation(Expression.AggregationInvocation.ALL)
              .lowerBound(WindowBound.Preceding.of(5))
              .upperBound(WindowBound.CURRENT_ROW)
              .boundsType(Expression.WindowBoundsType.RANGE)
              .build());
}

My other two points were about the description only — drop them if you'd rather not touch it.

[nielspardon]

Drop Closes #1198 rather than filing a follow-up for the deferred rule: #1198's own "What core does today" item 3 is exactly add(T, D) -> T going unchecked, and its Open Questions section already hosts the "resolving a function from inside POJO validation is a different weight class" discussion — so leaving #1198 open tracks the remainder better than a new issue would, and #1230 is the isthmus half already pointing back to it.

Two doc spots sit outside the diff hunks, so I couldn't anchor them inline: Preceding.of(long) and Following.of(long) still read "For BOUNDS_TYPE_ROWS only", which this PR's own new tests contradict — they pair Preceding.of(5) with RANGE over an R.I64 ordering column, and v0.102.0's site/docs/expressions/window_functions.md:34 names i64/i64 as an explicit valid pairing — and the no-sorts windowFn overload should say in its Javadoc that it supplies an empty ordering list and therefore rejects a RANGE Preceding/Following frame outright.

Nothing exercises the new sorts-carrying overload: all seven sb.windowFn( call sites still use the old one, and each is either ROWS or RANGE with UNBOUNDED bounds, so the single shape the overload was added to make expressible has no test behind it.

Unrelated to this PR but adjacent to the fixtures it touches: #1231 — RelCopyOnWriteVisitor.visit(ConsistentPartitionWindow) never visits its input, which the window CoW tests can't catch while their input is a bare namedScan.

[anasik]

@nielspardon understood. Dropped the Closes #1198

[nielspardon]

The rebase resolution is the right one — folding the ordering into windowOver covers its Preceding/Following caller and leaves the two UNBOUNDED/CURRENT_ROW callers unaffected. Two small things inline, neither blocking.

Worth putting the citation on the PR, since the body carries none: v0.102.0 states this rule verbatim at site/docs/expressions/window_functions.md:34, and again in proto/substrait/algebra.proto at :457 (WindowRelFunction.bounds_type) and :1401 (Expression.WindowFunction.sorts) — so the count and the CLUSTERED half are both spec-defined, and gating them on a Preceding/Following side matches the spec's own scope rather than over-applying to every RANGE frame the way substrait-go does.

The only clause the new Javadoc's paraphrase drops is "or a custom comparison function" from that same sentence, which Expression.SortField cannot express today — it has expr() and direction() and no comparison_function_reference — so that half stays with #1198 and needs nothing here.