Spark 3.5, 4.0: Prevent unnecessary failure when executing DML queries with identifier fields - #13435
Conversation
66ffdda to
9a9228c
Compare
9a9228c to
ccead30
Compare
|
FYI @manuzhang, @dramaticlly . Also @amogh-jahagirdar @huaxingao can you help take a look? |
dramaticlly
left a comment
There was a problem hiding this comment.
LGTM, Thanks @szehon-ho for the fix!
| return new Schema( | ||
| metaColumnFields, | ||
| table.schema().identifierFieldIds(), | ||
| Set.of(), |
There was a problem hiding this comment.
I think I agree we probably should not be passing through the identifier fields here but the new test added still passes even without this fix. Are we sure this is the right place or is there another cause? Or perhaps there are more specific conditions.
There was a problem hiding this comment.
thanks for looking, I think i removed a lot of my original test due to the earlier review comment, let me double check how to reproduce it.
There was a problem hiding this comment.
Got it, i think it needs to query the metadata column. I removed it as part of addressing Manu's comment in 49751dc and put it back.
There was a problem hiding this comment.
Ah I missed that too yeah the projection needs to include a metadata column, cool!
|
Thanks @szehon-ho , thanks @manuzhang @dramaticlly for reviewing. I'll go ahead and merge |
|
Will this PR be included in 1.10.0? @szehon-ho @manuzhang |
I believe both this and back port change to Spark 3.4 has been released in iceberg 1.10 https://github.com/apache/iceberg/releases/tag/apache-iceberg-1.10.0 |
Fixes #11341
This fixes a bug introduced in #10547, where metadata tables are broken for tables with identifier columns. Metadata schemas got the identifier columns of the parent table, but they should not have identifier columns.