peter-toth opened a new pull request, #59257:
URL: https://github.com/apache/spark/pull/59257
### What changes were proposed in this pull request?
`DataSourceV2ScanExecBase.outputOrdering` now derives an ordering from the
partition key expressions whenever the reported ordering keeps no sort order
over the scan output. Before, it derived one only when the scan reported no
ordering at all.
- It still restricts the reported ordering to the scan output first. If
nothing is left, the output partitioning is a `KeyedPartitioning` and
`spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` is on, it returns
the ordering over the partition keys.
- Two cases lost the key ordering before:
- an empty report, which `V2ScanPartitioningAndOrdering` stores as
`Some(Nil)`;
- a report that starts with a sort order on a pruned column and has no
sort order on a partition key, which the restriction (SPARK-59899) leaves empty.
- The conf doc and `sql-performance-tuning.md` name the pruned case too. An
empty report already counts as "no explicit ordering" there.
- A comment in `V2ScanPartitioningAndOrdering` said that only `None` lets
the derivation run. It is reworded.
- A comment in `GroupPartitionsExec.outputOrdering` said the scan prepends a
key-derived ordering. It derives one only when nothing is left of the report.
The comment is fixed.
- `DataSourceV2Suite`'s "ordering and partitioning reporting" now turns the
derivation off. Its source breaks the `KeyGroupedPartitioning` contract, e.g.
`[1, 1, 3]` under the key 1. The test checks the reported ordering and
partitioning. Its case with partitioning on `i` and no ordering expects a sort
under `groupBy(i)`, which the derived ordering removes.
The derived ordering can hold a partition transform, such as `years(ts)`.
The k-way merge of `GroupPartitionsExec` failed on one before SPARK-59995, so
this PR goes after it.
### Why are the changes needed?
With `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` on, the
default since SPARK-59396, a keyed scan reports an ordering over its partition
key expressions (SPARK-56241). The conf's doc covers a source that "reports a
KeyedPartitioning but does not report explicit ordering via
SupportsReportOrdering". A scan that implements `SupportsReportOrdering` and
returns an empty array lost that ordering. So did one whose report starts with
a pruned column and has no sort order on a partition key. A sort-merge join on
the partition key then kept a sort above the scan. The results were correct.
### Does this PR introduce _any_ user-facing change?
Yes. Such a scan now reports the ordering over its partition keys, so a plan
can drop a sort above it.
The derivation relies on the `KeyGroupedPartitioning` contract that every
row of a partition has the same partition value. A source that breaks it was
already exposed to a wrong derived ordering when it does not implement
`SupportsReportOrdering`. With this change, it is also exposed when it reports
an empty ordering, or one that keeps no sort order over the scan output.
### How was this patch tested?
New test in `KeyGroupedPartitioningSuite`, "SPARK-59981: a reported ordering
that keeps no sort order falls back to the keys". It joins a scan that reports
`identity(id)` with another `identity(id)` table. The scan reports three
orderings in turn: an empty one, one on `s`, which the join prunes, and `[s,
data]`, where `data` is not a partition key:
- with the conf on, the scan keeps the report as it is, its output ordering
is `id ASC`, and the join has no sort above it;
- with the conf off, there is no ordering and one sort.
It fails on master.
Also ran the `KeyGroupedPartitioning*` suites, `DataSourceV2Suite`, the plan
merging suites, `ProjectedOrderingAndPartitioningSuite`,
`GroupPartitionsExecSuite`, `PlannerSuite`,
`WriteDistributionAndOrderingSuite`, `EnsureRequirementsSuite`, the plan
stability suites, the `*JoinSuite` suites and `AdaptiveQueryExecSuite`, 2143
tests in all, plus `dev/lint-scala`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
--
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]
