andygrove opened a new pull request, #6735:
URL: https://github.com/apache/datafusion-comet/pull/6735
## Which issue does this PR close?
Part of #6385. Follow-up to #6683 and #6684, which both said this guard
could go once they landed.
## Rationale for this change
Strict floating-point mode declined a sort or window key that nests floats
in an array or struct whose type can hold a null element or field. Spark orders
such a null below every other value, whatever the key's null order. The native
sort placed it by the null order (#6476), and a `RANGE` frame ordered it above
every other value (#6477), so strict mode, which promises Spark's
floating-point results, fell back for every such key.
#6683 and #6684 now decline exactly the shapes where that differs, in every
mode: `ASC NULLS LAST` or `DESC NULLS FIRST` on such a key, and a `RANGE` frame
that has to find a row's peers over one. Everything else the strict-only
decline still caught already matches Spark: the default null orders in a Sort,
a TopK, ranking functions, a rank limit, `ROWS` frames, and a `RANGE` frame
bounded only by the partition. So strict mode no longer needs its own rule for
these keys.
## What changes are included in this PR?
- `CometSortOrder` drops the strict-only decline and its incompatible
reason. `canHoldNestedNull` stays, because both new guards use it.
- `windows/nested_float_order_keys_strict.sql` runs those shapes natively
under strict mode, over keys holding a null element, a `-0.0` and a NaN with
the sign bit set. It keeps `expect_fallback` for a non-default null order in a
sort and in a window, and for a `RANGE` frame that finds peers. A key that
can't hold a null now also gets a `NULLS LAST` case, which stays native.
- `CometWindowExecSuite` expects a native rank limit over `array(f)`,
`array(d)` and `named_struct('x', d)` in strict mode, where it used to expect a
fallback.
- The nested strict sort test in `CometExpressionSuite` adds a key whose
element can be null, and checks that it sorts natively and keeps NaN payloads
and zero signs.
- The floating-point compatibility page, the operator tuning page and the
`spark.comet.exec.strictFloatingPoint` doc no longer describe a strict
exception. They now say these shapes fall back in every mode.
## How are these changes tested?
Before removing the guard, I checked that every shape it still declined
either matches Spark natively or is declined by the #6683/#6684 guards. The
strict fixture now covers each one, and plain `query` asserts native execution
as well as Spark's answer.
- Spark 4.1: 568 passed. That covers the `windows/` and `operators/`
fixtures, `CometWindowExecSuite`, `CometTopKSuite`, `CometFloatSemanticsSuite`,
the strict floating-point tests in `CometExpressionSuite`, and the sort and
TopK tests in `CometExecSuite`.
- Spark 3.5: 91 passed, covering the `windows/` and `operators/` fixtures
and `CometWindowExecSuite`.
- Scalastyle and spotless ran in both builds, and prettier `--check` passes
on both docs.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
