Repository navigation
[SPARK-53546][SQL][TESTS] Fix InMemoryDataSource to return default value or null for new fields - #52299
Closed
szehon-ho wants to merge 3 commits into
Closed
[SPARK-53546][SQL][TESTS] Fix InMemoryDataSource to return default value or null for new fields#52299szehon-ho wants to merge 3 commits into
szehon-ho wants to merge 3 commits into
Conversation
szehon-ho
commented
Sep 11, 2025
Member
Author
There was a problem hiding this comment.
this method is used by alterTable and regular write, so we have a flag newData=false to preserve the original 'writeSchema' for the BufferedRows in the alterTable case instead of using the new schema. In the write case, we mark the new rows with the new schema.
Member
Author
There was a problem hiding this comment.
I end up, making another method for alterTable to not pollute existing logic with if-else. That version preserves the 'writeSchema' on the BufferedRows.
…lue or null for new fields
szehon-ho
force-pushed
the
in_memory_schema_evolution
branch
from
September 11, 2025 00:41
6f66699 to
616f350
Compare
cloud-fan
reviewed
Sep 11, 2025
szehon-ho
commented
Sep 12, 2025
Member
Author
There was a problem hiding this comment.
Note: Array[Column] is not serializable, so hence passing StructType to these BatchWriterFactory and BatchWriters
szehon-ho
force-pushed
the
in_memory_schema_evolution
branch
from
September 12, 2025 22:24
5357f86 to
a1dc29c
Compare
cloud-fan
approved these changes
Sep 15, 2025
Contributor
|
thanks, merging to master! |
cloud-fan
reviewed
Sep 15, 2025
Comment on lines
+354
to
+355
| def alterTableWithData(data: Array[BufferedRows], | ||
| newSchema: StructType): InMemoryBaseTable = { |
Contributor
There was a problem hiding this comment.
style nit:
Suggested change
| def alterTableWithData(data: Array[BufferedRows], | |
| newSchema: StructType): InMemoryBaseTable = { | |
| def alterTableWithData( | |
| data: Array[BufferedRows], | |
| newSchema: StructType): InMemoryBaseTable = { |
Contributor
There was a problem hiding this comment.
You can fix them in your next PR.
cloud-fan
pushed a commit
that referenced
this pull request
Sep 17, 2025
… style for InMemoryBaseTable ### What changes were proposed in this pull request? Fix nested array struct schema evolution case for InMemoryDataSource, also style fixes. ### Why are the changes needed? Address style review comment (#52299 (comment)) and a small bug fix for #52299 (test-only) ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Ran affected unit test ### Was this patch authored or co-authored using generative AI tooling? No Closes #52359 from szehon-ho/in_memory_schema_evolution_fix. Authored-by: Szehon Ho <szehon.apache@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
huangxiaopingRD
pushed a commit
to huangxiaopingRD/spark
that referenced
this pull request
Nov 25, 2025
…lue or null for new fields ### What changes were proposed in this pull request? InMemoryTable does not handle schema evolution properly. In particular, it always uses the current schema to read the in-memory rows. For newly-added columns that don't exist in the row, the InternalRow sometimes return some junk value. This fixes InMemoryDataSource to properly handle schema evolution. Like modern table format, it persists the 'writeSchema' used to write the BufferedRows (on the BufferedRows object here), and then at read time resolves it with the 'readSchema'. ### Why are the changes needed? While writing tests for MERGE INTO schema evolution, realized that in these cases InMemoryTable is returning junk values, unlike a real V2 data source. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Add a new test in AlterTableTests ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#52299 from szehon-ho/in_memory_schema_evolution. Authored-by: Szehon Ho <szehon.apache@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
huangxiaopingRD
pushed a commit
to huangxiaopingRD/spark
that referenced
this pull request
Nov 25, 2025
… style for InMemoryBaseTable ### What changes were proposed in this pull request? Fix nested array struct schema evolution case for InMemoryDataSource, also style fixes. ### Why are the changes needed? Address style review comment (apache#52299 (comment)) and a small bug fix for apache#52299 (test-only) ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Ran affected unit test ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#52359 from szehon-ho/in_memory_schema_evolution_fix. Authored-by: Szehon Ho <szehon.apache@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
InMemoryTable does not handle schema evolution properly. In particular, it always uses the current schema to read the in-memory rows. For newly-added columns that don't exist in the row, the InternalRow sometimes return some junk value.
This fixes InMemoryDataSource to properly handle schema evolution. Like modern table format, it persists the 'writeSchema' used to write the BufferedRows (on the BufferedRows object here), and then at read time resolves it with the 'readSchema'.
Why are the changes needed?
While writing tests for MERGE INTO schema evolution, realized that in these cases InMemoryTable is returning junk values, unlike a real V2 data source.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Add a new test in AlterTableTests
Was this patch authored or co-authored using generative AI tooling?
No