Repository navigation
Commit 751a496
[SPARK-59721][SQL] Ignore reported partitioning and ordering that reference unresolvable columns
### What changes were proposed in this pull request?
When a V2 scan reports a `KeyGroupedPartitioning` or an `ordering` that references a column the relation can't resolve, the query used to fail with `Unable to resolve X given [...]`. This PR makes `V2ScanPartitioningAndOrdering` check the columns referenced by the reported partition keys and ordering (recursively, including nested transform arguments) against the relation before converting them. If any can't be resolved, including a missing nested field, an ambiguous name, or an empty name, the rule logs a warning naming the relation, the scan class, and those columns, and ignores the report. The partitioning falls back to `UnknownPartitioning`, and the ordering is dropped.
This PR adds a `resolveRefOpt`, and makes `resolveRef` a thin wrapper around it. `resolveRefOpt` still throws for a missing nested field or an ambiguous name, so the rule treats those errors as unresolvable and includes the error condition in the warning. The `toCatalyst*` methods are unchanged.
Keys whose columns all resolve take the existing conversion path unchanged, so an unsupported expression type (e.g. `id + 1`) or a nested transform whose function can't be loaded still fails the query as before. Because the whole report is checked before any key is converted, a report that also references an unresolvable column falls back with a warning instead, e.g. `[id + 1, identity(missing)]`.
The Javadoc of `KeyGroupedPartitioning#keys()` and `SupportsReportOrdering#outputOrdering()` now says that Spark resolves the column references against the table columns, including columns pruned from the scan output, and ignores the whole report if any of them can't be resolved. The docs of `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` now mention that an ignored ordering can be replaced by one derived from the partition keys.
### Why are the changes needed?
Found while reviewing SPARK-58111 ([PR #55518](#55518)). This is a general, pre-existing bug in how `V2ScanPartitioningAndOrdering` handles reported partitioning and ordering, not something introduced by #55518.
### Does this PR introduce _any_ user-facing change?
Yes. A query against a data source whose scan reports a `KeyGroupedPartitioning` with a key that can't be resolved previously failed outright. It now succeeds, with partitioning degraded to `UnknownPartitioning` instead, and Spark logs a warning naming the relation, the scan class, and the columns that can't be resolved.
The same applies to an ambiguous or empty column name. A report that combines an unsupported expression with an unresolvable column, e.g. `missing + 1` or `[id + 1, identity(missing)]`, used to fail with `_LEGACY_ERROR_TEMP_3054` and now falls back with a warning. An unsupported expression over resolvable columns still fails as before.
If the scan keeps its partitioning but its reported ordering is ignored, Spark derives the ordering from the partition keys when `spark.sql.sources.v2.bucketing.partitionKeyOrdering.enabled` is on (the default).
### How was this patch tested?
New tests in `V2ExpressionUtilsSuite`:
- `resolveRefOpt` returns `None` for an unresolvable reference.
- `resolveRefOpt` throws `FIELD_NOT_FOUND` for a missing nested field and `AMBIGUOUS_REFERENCE` for an ambiguous reference.
New tests in `KeyGroupedPartitioningSuite`:
- A join against a table whose scan reports unresolvable partition keys succeeds, plans a shuffle instead of a storage-partitioned join, and logs a warning naming the relation, the scan class, and only the unresolvable columns. The cases are:
- a `bucket` key, a nested transform key, and a multi-key partitioning with one bad key;
- duplicate references, and two distinct unresolvable columns;
- a missing nested field;
- an unsupported expression next to or over an unresolvable column (`[id + 1, identity(missing)]`, `missing + 1`);
- a connector-defined transform that hides the reference from `children()`.
Resolvable keys (`id`, case-insensitive `ID`, and a connector-defined transform with an extra reference only in `children()`) keep the partitioning and plan a storage-partitioned join without a shuffle.
- A reported ordering that can't be resolved is ignored as a whole with a warning, and the scan still derives `id ASC` from its kept partitioning. The cases are a column that doesn't exist, one hidden from a connector-defined sort order's `children()`, an empty name, and a two-key ordering with one unresolvable key (`[id, missing]`). Resolvable orderings (`id`, case-insensitive `ID`, the metadata column `index`, and a connector-defined sort order with an extra reference only in `children()`) are kept without a warning.
- A reported partition key of an unsupported expression type (`id + 1`), or a nested transform whose function can't be loaded (`f(g(id))`), still fails the query with `_LEGACY_ERROR_TEMP_3054`.
New test in `DataSourceV2Suite`:
- An unresolvable ordering reported through connector-defined `SortOrder`, `Transform`, and `NamedReference` classes (`OrderAndPartitionAwareDataSource` and its Java counterpart) is ignored with a warning, and the scan relation keeps no ordering.
Also ran `V2ExpressionUtilsSuite`, `KeyGroupedPartitioningSuite`, `WriteDistributionAndOrderingSuite`, `DataSourceV2Suite`, `MergeSubplansSuite` (comment update only), `SQLConfSuite`, and `ProjectedOrderingAndPartitioningSuite` to confirm no regression in the partitioning/SPJ, ordering, and write paths this change touches.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Sonnet 5, Opus 5.5)
Verified manually by me.
Closes #58979 from anuragmantri/v2expr-opt-resolve-degrade.
Authored-by: Anurag Mantripragada <amantripragada@apple.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 9de605f)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>1 parent d8d72d2 commit 751a496
12 files changed
Lines changed: 409 additions & 44 deletions
File tree
- docs
- sql
- catalyst/src
- main
- java/org/apache/spark/sql/connector/read
- partitioning
- scala/org/apache/spark/sql
- catalyst/expressions
- internal
- test/scala/org/apache/spark/sql/catalyst/expressions
- core/src
- main/scala/org/apache/spark/sql/execution
- datasources/v2
- planmerging
- test/scala/org/apache/spark/sql
- connector
- execution/planmerging
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
708 | 708 | | |
709 | 709 | | |
710 | 710 | | |
711 | | - | |
| 711 | + | |
712 | 712 | | |
713 | 713 | | |
714 | 714 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
38 | 42 | | |
39 | 43 | | |
40 | 44 | | |
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
46 | 50 | | |
47 | 51 | | |
48 | 52 | | |
| |||
Lines changed: 15 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
49 | 59 | | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
58 | 65 | | |
59 | 66 | | |
60 | 67 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2590 | 2590 | | |
2591 | 2591 | | |
2592 | 2592 | | |
2593 | | - | |
| 2593 | + | |
| 2594 | + | |
2594 | 2595 | | |
2595 | 2596 | | |
2596 | 2597 | | |
| |||
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/V2ExpressionUtilsSuite.scala
Lines changed: 27 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
40 | 66 | | |
Lines changed: 9 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
147 | 147 | | |
148 | 148 | | |
149 | 149 | | |
| |||
Lines changed: 69 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
| 21 | + | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| 26 | + | |
| 27 | + | |
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
| |||
47 | 50 | | |
48 | 51 | | |
49 | 52 | | |
50 | | - | |
51 | | - | |
52 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
53 | 66 | | |
54 | 67 | | |
55 | 68 | | |
| |||
75 | 88 | | |
76 | 89 | | |
77 | 90 | | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
85 | 143 | | |
86 | 144 | | |
Lines changed: 7 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
980 | 980 | | |
981 | 981 | | |
982 | 982 | | |
983 | | - | |
984 | | - | |
985 | | - | |
986 | | - | |
987 | | - | |
988 | | - | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
| 988 | + | |
| 989 | + | |
989 | 990 | | |
990 | 991 | | |
991 | 992 | | |
| |||
Lines changed: 29 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
| 27 | + | |
27 | 28 | | |
28 | 29 | | |
29 | 30 | | |
| |||
397 | 398 | | |
398 | 399 | | |
399 | 400 | | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
400 | 429 | | |
401 | 430 | | |
402 | 431 | | |
| |||
0 commit comments