-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix reading map value contains variant null value #26700
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
Conversation
Previously, a `null` value was appended when the `VARIANT` block had zero positions. This caused a mismatch between key and value block sizes, leading to errors when building map types.
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.
Pull Request Overview
This PR fixes a bug where reading null map values with VARIANT as the value type would fail due to a mismatch between key and value block sizes. The fix removes problematic null value appending in the VARIANT reader when there are zero positions.
- Removes null appending logic in VARIANT reader that caused block size mismatches
- Updates test data to include map columns with VARIANT values for better coverage
- Expands test cases to verify the fix handles null maps with VARIANT values correctly
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
ParquetReader.java | Removes problematic null appending logic in readVariant method |
TestDeltaLakeBasic.java | Adds comprehensive test coverage for null maps with VARIANT values |
test_variant_null/_delta_log/*.json | Updates test metadata to include map columns with VARIANT values |
test_variant_null/README.md | Documents updated test data structure with map column |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/trino-parquet/src/main/java/io/trino/parquet/reader/ParquetReader.java
Show resolved
Hide resolved
"(4, NULL, NULL)," + | ||
"(5, JSON '{\"a\":5}', NULL)"); |
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.
The indentation at L1647 is wrong.
@chenjian2664 When fixing a query failure, please start the sentence with "Fix failure …". There are some common patterns for release note entries. Also, replace |
Previously, a
null
value was appended when theVARIANT
block had zero positions. This caused a mismatch between key and value block sizes, leading to errors when building map types.Description
When reading the null map value and the value type is a
variant
type, we will encounter:Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: