fix(parquet): allow reading nested list/map columns whose leaf types differ only in representation - #172
Draft
zhf999 wants to merge 1 commit into
Draft
fix(parquet): allow reading nested list/map columns whose leaf types differ only in representation#172zhf999 wants to merge 1 commit into
zhf999 wants to merge 1 commit into
Conversation
…differ only in representation
zhf999
marked this pull request as draft
August 3, 2026 05:24
lxy-9602
reviewed
Aug 3, 2026
| } | ||
|
|
||
| TEST_F(ParquetFileBatchReaderTest, TestNestedListTimestampTimezoneAndMapFieldName) { | ||
| const std::string timezone = "Asia/Shanghai"; |
Contributor
There was a problem hiding this comment.
Could we extend the test coverage for both newly supported nested timestamp projection cases?
First, please add a focused ParquetFileBatchReaderTest for a nested MILLI -> SECOND conversion, for example with list<struct<..., timestamp(SECOND)>>. The existing test only covers the nested MICRO timezone-only case and does not exercise the unit-conversion branch allowed by this guard.
In addition, please add E2E write-and-read cases for both scenarios in write_and_read_inte_test.cpp:
- nested MILLI -> SECOND unit conversion;
- nested MICRO timestamp with a timezone-only difference.
This would verify both the reader-level conversion logic and the complete Parquet write/read path.
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
Reading a Parquet column of type
list<struct<... timestamp(us, <tz>)>>(or amapwhose valuefield is not literally named
value) failed with:even though no partial projection was requested.
ParquetFileBatchReader::CollectLeafIndices()guards against pruning fields inside LIST/MAP byrequiring
read_type->Equals(file_type). That guard is too strict, because the two types beingcompared come from different layers:
file_typecomes fromFileReaderWrapper::GetSchema(), i.e. the raw Arrow schema derived fromthe Parquet metadata.
read_typeis the Paimon-facing read schema. Even the default self-read path inParquetFileBatchReader::Create()feeds backGetFileSchema(), which has already been rewrittenby
ParquetTimestampConverter::AdjustTimezone().So
Equals()rejects pure representation differences that the reader is designed to reconcilelater in
NextBatch()viaParquetTimestampConverter::NeedCastArrayForTimestamp()/CastArrayForTimestamp():UTC, while Paimon exposesthem in the local timezone.
cast back to SECOND on read.
attrsin the file versus thevalueproduced byarrow::map(), alistelement field name, or a differing nullable flag.This PR makes the guard compare only the projection shape:
HasSameNestedProjectionShape()insrc/paimon/format/parquet/parquet_file_batch_reader.cpp:value_type()only, ignoring the element field name and nullability.key_type()anditem_type()only, ignoring the key/value field names.type is SECOND — exactly the conversions
ParquetTimestampConvertersupports. Timezonedifferences are always accepted and left to the cast path.
Equals()check, and a nested-versus-atomic mismatch isrejected.
CollectLeafIndices()splits the shared LIST/MAP branch and now recurses throughListType::value_type()andMapType::key_type()/item_type()instead of iteratingfile_type->field(i). This also fixes a latent leaf-index bug:arrow::MapType::num_fields()is 1 (the
entriesstruct), so the old code fell into the STRUCT branch and matched map childrenby name; when the file's value field name differed from the read schema's, it took the
SkipLeafIndices()path and silently produced a wrong leaf index list.Fail-fast behavior for genuinely unsupported cases is preserved: pruning a field from a struct
inside a list/map, incompatible leaf types, and unsupported timestamp unit conversions all still
return
Status::Invalid.Tests
New case in target
paimon-parquet-format-test:ParquetFileBatchReaderTest.TestNestedListTimestampTimezoneAndMapFieldName(
src/paimon/format/parquet/parquet_file_batch_reader_test.cpp) writeslist<struct<key: utf8, attrs: map<utf8, utf8 ("attrs" value field)>, updated_at: timestamp(us, Asia/Shanghai)>>under aTimezoneGuard, including empty maps, null maps and null timestamps,then asserts:
Status::Invalid);updated_atfrom the nested struct still fails with"Parquet does not support partial projection inside list/map";updated_atasutf8still fails;updated_atastimestamp(ns, ...)(an unsupported unit conversion) still fails.API and Format
No changes.
Documentation
No changes.
Generative AI tooling
No.