peter-toth commented on code in PR #59199:
URL: https://github.com/apache/spark/pull/59199#discussion_r4193997614
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanPartitioningAndOrdering.scala:
##########
@@ -104,8 +104,12 @@ object V2ScanPartitioningAndOrdering extends
Rule[LogicalPlan] with Logging {
// scan output: truncating it here would also drop the sort orders on
a partition key past a
// pruned column, which still hold.
`DataSourceV2ScanRelation.doCanonicalize` and
// `DataSourceV2ScanExecBase.outputOrdering` restrict it to the scan
output instead.
+ // A nested sort key such as `s.x` resolves to a new
`Alias(GetStructField(...))` on every
+ // run of this rule. Strip the alias so repeated runs give equal plans
and the ordering
+ // can satisfy a required ordering on the same field.
val ordering =
V2ExpressionUtils.toCatalystOrdering(reportedOrdering, relation,
relation.funCatalog)
+ .map(o => o.copy(child = trimAliases(o.child)))
Review Comment:
**Finding 1.** This fixes the case where the scan keeps the whole struct,
but not when nested schema pruning narrows it. I measured it at this head with
`s struct<x: bigint, y: bigint>` and a reported ordering on `s.x`:
| query | scan output `s` | sorts |
|---|---|---|
| `SELECT * FROM t SORT BY s.x` | `struct<x,y>` | 0 |
| `SELECT id, s FROM t SORT BY s.x` | `struct<x,y>` | 0 |
| `SELECT id, s.x FROM t SORT BY s.x` | `struct<x>` | 1 |
With `s struct<x>` only, the third query has no sort, so the pruning is what
keeps it.
The ordering is resolved against `relation`, the full
`DataSourceV2Relation`, so its `s` attribute keeps the unpruned type. The
required ordering reads `s` from the scan output, which has the pruned type.
`AttributeReference` equality includes the data type, so the two
`GetStructField`s are never semantically equal. The ticket's own repro, `SELECT
s.x FROM t`, has this shape once the struct has a second field.
Rebinding the attribute to the scan output by `exprId` would not be enough
on its own, because the `GetStructField` ordinal also comes from the full
struct. So I think this is fine as a follow-up. It would help to say in the
comment and in the description that stripping the alias only helps while the
struct is not narrowed.
--
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]
