-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-52860][SQL][Test] Support V2 write schema evolution in InMemoryTable #51506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
@cloud-fan @dongjoon-hyun @huaxingao can one of you take a look? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @szehon-ho . Could you file a JIRA issue for traceability?
|
||
// this does not override the old field with the new field with same name for now | ||
val nameToFieldMap = oldFields.map (f => f.name -> f).toMap | ||
val remainingNewFields = newFields.filterNot (f => nameToFieldMap.contains (f.name) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the code, do we assume case-sensitive column names always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea its up to the DSV2 implementation, for example Iceberg uses a config [1], which actually is the spark config spark.sql.caseSensitive
[2]. ref:
- https://github.com/apache/iceberg/blob/main/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/SparkWriteBuilder.java#L199
- https://github.com/apache/iceberg/blob/main/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java#L456
I was just making a simple example for InMemoryTable, should I can make it more complex and take this property into account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK i added this support as well in InMemoryTable, thanks!
What changes were proposed in this pull request?
Add test for DSV2 Write schema evolution for InMemoryTable, by making it support TableCapability.ACCEPT_ANY_SCHEMA.
Why are the changes needed?
There is a test on the analysis phase in V2WriteAnalysisSuite to make sure it skips the output resolution if this flag is enabled. But it is not end-to-end test for this feature (schema evolution on V2Write). For authors of DSV2 connector, its better to have an end to end test using the demo InMemoryTable to show to how to use this feature.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Ran the new test
Was this patch authored or co-authored using generative AI tooling?
No