JingsongLi commented on code in PR #9415:
URL: https://github.com/apache/paimon/pull/9415#discussion_r4052061926
##########
paimon-spark/paimon-spark-common/src/main/scala/org/apache/paimon/spark/catalyst/analysis/PaimonOutputResolver.scala:
##########
@@ -58,6 +58,35 @@ object PaimonOutputResolver extends SQLConfHelper {
import MissingFieldBehavior._
+ def renameNestedFieldsByPosition(
+ query: LogicalPlan,
+ expected: Seq[Attribute],
+ queryOutputByName: Boolean): LogicalPlan = {
+ val renamed = query.output.zipWithIndex.map {
+ case (input, index) =>
+ val target = if (queryOutputByName) {
+ expected.find(target => conf.resolver(input.name, target.name))
+ } else {
+ expected.lift(index)
Review Comment:
[P1] Partial column lists silently misplace values on Spark 3.2/3.3
`expected.lift(index)` assumes `query.output` is already aligned with the
table columns, which only holds on Spark 3.4+ (there `addColumnListOnQuery`
fills the omitted columns, SPARK-42521). On Spark 3.2/3.3
`ResolveUserSpecifiedColumns.addColumnListOnQuery` keeps only the listed
columns, re-ordered into table order (`tableOutput.flatMap {
nameToQueryExpr.get(_).orElse(None) }`), so the ordinal no longer matches the
table schema whenever the column list skips a column that precedes a listed
one. `applyColumnMetadata` then aliases the value to the wrong table column,
and `effectiveByName = true` NULL-fills the real target.
Reproduced on the current head (00955d7) with `INSERT ... SELECT`:
```
CREATE TABLE p1 (a INT, b INT, c INT);
INSERT INTO p1 (a, c) SELECT 1, 3; -- 3.2/3.3 store [1, 3, null], expected
[1, null, 3]
CREATE TABLE p6 (a INT, b INT, c INT);
INSERT INTO p6 (c) SELECT 3; -- 3.2/3.3 store [3, null, null],
expected [null, null, 3]
```
and with a gap column of a different type the insert fails with a misleading
error:
```
CREATE TABLE p4 (a INT, s STRUCT<x: INT, y: INT>, c INT);
INSERT INTO p4 (a, c) SELECT 5, 7; -- AnalysisException: cannot resolve
's' due to data type mismatch: cannot cast int to struct<x:int,y:int>
```
Spark 3.4/3.5 handle the same statements correctly (`[1, null, 3]`, `[null,
null, 3]`, `[5, null, 7]`).
This is a regression: on Spark 3.2/3.3 these statements used to be rejected
loudly with `Cannot write to \`p1\`, the number of data columns (2) doesn't
match the table schema's (3).` (the existing test comment in
`InsertOverwriteTableTestBase` — "In Spark 3.3 or earlier, these commands would
have failed" — documents that intent). The PR converts a clear failure into
silently corrupted rows, and `INSERT INTO p1 (a, c) VALUES (1, 3)` still raises
the old error on 3.2/3.3 while the `SELECT` form corrupts, so the semantics now
depend on the query shape. The added tests all use full column lists, so
nothing covers this.
Suggested fix: only take the positional nested-field path when the query
output is known to be full width and in table order — e.g. guard the call with
`query.output.size == table.output.size` and keep the previous by-position
behavior (loud error) otherwise, or map each input column to the column it was
actually listed as instead of `expected.lift(index)`. A regression test with a
partial column list on Spark 3.2/3.3 would pin this down.
--
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]
