peter-toth commented on code in PR #58681:
URL: https://github.com/apache/spark/pull/58681#discussion_r4057205314


##########
docs/sql-migration-guide.md:
##########
@@ -34,6 +34,9 @@ license: |
 - Since Spark 4.4, the CSV `extension` option must be non-empty and contain 
only letters. In Spark 4.3 and earlier, due to a bug in the extension 
validation check, empty strings and three-character suffixes containing 
non-letters (for example `ab1`) were accepted. This affects both writes and 
reads of CSV tables whose persisted `OPTIONS` contain such values; update the 
option to a non-empty letters-only suffix to restore access.
 - Since Spark 4.4, when `array_repeat` or `array_insert` is asked to build an 
array larger than the maximum supported array length, generated code raises the 
same error as interpreted evaluation. `array_repeat` now fails with 
`COLLECTION_SIZE_LIMIT_EXCEEDED.PARAMETER` instead of the internal error 
`_LEGACY_ERROR_TEMP_2176`, and `array_insert` fails with 
`COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` instead of 
`COLLECTION_SIZE_LIMIT_EXCEEDED.PARAMETER`, which named a `count` parameter 
that `array_insert` does not have. Both functions raise an error under exactly 
the same conditions as before; only the reported error condition changes.
 - Since Spark 4.4, `/*+ ... */` inside a bracketed comment is parsed as a 
nested comment, so its closing `*/` no longer closes the outer comment. For 
example, `/* note /*+ x */ SELECT 1` previously returned `1`, but now raises 
`UNCLOSED_BRACKETED_COMMENT`. Close the outer comment explicitly, for example 
`/* note /*+ x */ */ SELECT 1`.
+- Since Spark 4.4, `spark.sql.sources.v2.bucketing.partition.filter.enabled` 
defaults to `true`. In a storage-partitioned join, the partition key groups 
pushed down to both sides may now be narrowed to those that can produce output 
for the join type, instead of always taking the union of the two sides' groups: 
an inner or semi join may keep only the groups present on both sides, a join 
that keeps or tests every left row (left outer, left anti, left single, 
existence) keeps the left side's groups, a right outer join keeps the right 
side's, and a full outer join is unaffected; a CROSS join carrying an equality 
condition is treated like an inner join. The narrowing is skipped when either 
side's partitioning may contain unknown partition keys, as after a shuffle on 
one side, so such a join keeps the full union. A group that is dropped is not 
scanned at all, so a query may read fewer files. To restore the previous 
behavior, set `spark.sql.sources.v2.bucketing.partition.filter.enabled` to
  `false`.
+- Since Spark 4.4, 
`spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` defaults to 
`true`. A V2 scan that reports a keyed partitioning but no explicit ordering 
now also reports itself sorted by its partition key expressions, because every 
row of such a partition evaluates them to the same value. Spark can then drop a 
`Sort` it would otherwise place above the scan. To restore the previous 
behavior, set `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` to 
`false`.

Review Comment:
   **Finding 10.** A user whose plan changed under these defaults will most 
often be looking at an aggregate, not at a `Sort`. All three entries and the 
description's user-facing section name the visible change as a `Sort` 
disappearing. There is a second one: 
`spark.sql.execution.replaceHashWithSortAgg` is on by default and keys off 
`child.outputOrdering`, so once the scan reports its key expressions an 
aggregate whose grouping is exactly those expressions is planned as a sort 
aggregate.
   
   Measured on `a3ad406`. `items` partitioned by `identity(id)` holding ids `1, 
1, 2, 3`, `SELECT id, count(*) FROM items GROUP BY id`, AQE off, only these two 
configs varying. The rows are identical in all four arms:
   
   | `partitionKeyOrdering` | `preserveKeyOrderingOnCoalesce` | aggregates |
   |---|---|---|
   | off | off | 2 hash |
   | **on** | off | 1 hash, 1 sort |
   | off | on | 2 hash |
   | **on** | **on** | 2 sort |
   
   The partial aggregate reads the scan, so this entry alone moves that one. 
The final aggregate reads the `GroupPartitionsExec` that coalesces the two `id 
= 1` splits, so the next entry moves that one. `SELECT DISTINCT id` behaves the 
same way. No sort is added in any arm, and with one key value per partition the 
sort aggregate is the cheaper shape, so this is not a regression. It is simply 
the plan diff a user is most likely to open the guide to explain.
   
   One clause on this entry would cover it, for example after "Spark can then 
drop a `Sort` it would otherwise place above the scan.":
   
   > An aggregate whose grouping is exactly the partition key expressions may 
also be planned as a sort aggregate rather than a hash aggregate, because 
`spark.sql.execution.replaceHashWithSortAgg` keys off the reported ordering.
   



-- 
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]

Reply via email to