[parquet] Fix timestamp type and decimal type, if the file schema is not correctly match the schema in metadata - #5582
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues with timestamp and decimal type conversion when the file schema does not match the metadata by updating converter logic and adding test coverage.
- Added tests that exercise conversions for both TIMESTAMP and DECIMAL types.
- Modified lazy decoding checks in the vectorized reader to rely on the actual column vector type.
- Refactored the timestamp and decimal updater implementations to use precision-dependent logic.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| paimon-format/src/test/java/org/apache/paimon/format/parquet/reader/FileTypeNotMatchReadTypeTest.java | Added tests for timestamp and decimal conversion with variable precisions. |
| paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/VectorizedColumnReader.java | Updated lazy decoding support to use the concrete column vector type. |
| paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/ParquetVectorUpdaterFactory.java | Refactored timestamp and decimal updaters with precision logic and updated type parameters. |
| paimon-format/src/main/java/org/apache/paimon/format/parquet/reader/ParquetSplitReaderUtil.java | Removed unnecessary checkArgument conditions and cleaned up import statements. |
| } | ||
| } | ||
|
|
||
| protected void putDecimal(WritableColumnVector values, int offset, BigDecimal decimal) { |
There was a problem hiding this comment.
The putDecimal utility method is defined within the TimestampUpdater block but is used by other decimal updater classes. Consider moving this method to a shared utility class or the enclosing ParquetVectorUpdaterFactory to improve reuse and maintainability.
| for (int i = 0; i < 100; i++) { | ||
| int writePrecision = RANDOM.nextInt(10); | ||
| int readPrecision = writePrecision == 0 ? 0 : RANDOM.nextInt(writePrecision); | ||
|
|
There was a problem hiding this comment.
[nitpick] It would be helpful to add a brief comment explaining the rationale for forcing readPrecision to 4 when readPrecision is less than or equal to 3 and writePrecision is greater than 3, clarifying the test’s intent.
Suggested change
| // If the write precision is greater than 3 but the read precision is 3 or less, | |
| // we force the read precision to 4. This ensures the test covers scenarios where | |
| // the read precision exceeds the default precision for timestamps (3), which | |
| // might be necessary for compatibility or edge case testing. |
Contributor
|
+1 |
JingsongLi
pushed a commit
that referenced
this pull request
May 9, 2025
…not correctly match the schema in metadata (#5582)
This was referenced Sep 3, 2026
Closed
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
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.
Purpose
If the timestamp field, in paimon, precision is 3. But in file, it is Binary. We will get an exception.
Fix this issue, do the converter for timestamp type and decimal type.
Tests
API and Format
Documentation