HyukjinKwon commented on code in PR #59463: URL: https://github.com/apache/spark/pull/59463#discussion_r4281509812
########## docs/sql-migration-guide.md: ########## @@ -48,6 +48,7 @@ license: | - 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. Partition transforms such as `days(ts)` or `bucket(8, id)` are left out of that ordering. This lets Spark drop a `Sort` it would otherwise place above the scan. An aggregate that groups by a leading run of the other key expressions, in key order, may also be planned as a sort aggregate rather than a hash aggregate. This is because `spark.sql.execution.replaceHashWithSortAgg` keys off the reported ordering. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` to `false`. - Since Spark 4.4, `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` defaults to `true`. When `GroupPartitionsExec` merges several input partitions that share one partition key value into a single output partition, it now keeps sort orders over the partition key expressions in the ordering it reports, since those expressions are constant within the merged partition. Sort orders over partition transforms such as `days(ts)` or `bucket(8, id)` are left out. Orders over other columns are still dropped, as the concatenation invalidates them, and a join that reduced the partition keys onto a common key space (see `spark.sql.sources.v2.bucketing.allowCompatibleTransforms.enabled`) reports no order, since the merged partitions then share only the reduced key. This can remove a `Sort` downstream of the merge. To restore the previous behavior, set `spark.sql.sources.v2.bucketing.preserveKeyOrderingOnCoalesce.enabled` to `false`. - Since Spark 4.4, the value columns of `UNPIVOT` and `Dataset.unpivot` merge the nested nullability (array element, map value, and struct field nullability) of all unpivoted values. Previously, a value column took the nested nullability of the first listed value, so a `NULL` array element, map value, or struct field of a later value could be returned as a non-`NULL` value such as `0`, and `IS NULL` on it returned `false`. A view created by an earlier Spark version over such an `UNPIVOT` may have recorded the non-nullable nested type in its schema and can fail to read; recreate it with `CREATE OR REPLACE VIEW`. To restore the previous behavior, set `spark.sql.legacy.unpivot.useFirstValueNestedNullability` to `true`. +- Since Spark 4.4, the pipe `AGGREGATE` operator rejects an aliased integer literal in its `GROUP BY` clause, such as `TABLE t |> AGGREGATE sum(x) AS s GROUP BY 2 AS grp`, with `PARSE_SYNTAX_ERROR`, as a regular `GROUP BY` clause does. Previously the aliased literal was grouped as a constant, while a bare integer literal in the same position is an ordinal. Bare ordinals and aliases on other grouping expressions are not affected. To restore the previous behavior, set `spark.sql.legacy.allowAliasedPipeAggregateOrdinals` to `true`. Review Comment: **Nit (P3):** This note records the new exclusion, but the operator's reference page, `docs/sql-pipe-syntax.md`, still says it is allowed. The AGGREGATE section shows `GROUP BY <grouping_expr> [AS alias], ...`, says "Aliases can be assigned directly on grouping expressions", and separately says "The `GROUP BY` expressions may include one-based ordinals" (the overview near line 112 says the same). Nothing says an ordinal cannot take an alias. Could you add a sentence next to the ordinals paragraph saying that an integer-literal ordinal cannot be aliased, so `GROUP BY 2 AS grp` is a syntax error as in regular `GROUP BY`, and keep bare ordinals and aliases on other grouping expressions described as supported? **Verification:** - **Inspection:** The AGGREGATE section of the pipe-syntax reference names the aliased-ordinal exclusion in a way consistent with the parser behavior and the migration-guide note, and leaves bare ordinals and aliases on other grouping expressions described as supported. -- 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]
