Skip to content

fix(parquet): allow reading nested list/map columns whose leaf types differ only in representation - #172

Draft
zhf999 wants to merge 1 commit into
apache:mainfrom
zhf999:timestamp-fix
Draft

fix(parquet): allow reading nested list/map columns whose leaf types differ only in representation#172
zhf999 wants to merge 1 commit into
apache:mainfrom
zhf999:timestamp-fix

Conversation

@zhf999

@zhf999 zhf999 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Purpose

Reading a Parquet column of type list<struct<... timestamp(us, <tz>)>> (or a map whose value
field is not literally named value) failed with:

Parquet does not support partial projection inside list/map: src ... vs target ...

even though no partial projection was requested.

ParquetFileBatchReader::CollectLeafIndices() guards against pruning fields inside LIST/MAP by
requiring read_type->Equals(file_type). That guard is too strict, because the two types being
compared come from different layers:

  • file_type comes from FileReaderWrapper::GetSchema(), i.e. the raw Arrow schema derived from
    the Parquet metadata.
  • read_type is the Paimon-facing read schema. Even the default self-read path in
    ParquetFileBatchReader::Create() feeds back GetFileSchema(), which has already been rewritten
    by ParquetTimestampConverter::AdjustTimezone().

So Equals() rejects pure representation differences that the reader is designed to reconcile
later in NextBatch() via ParquetTimestampConverter::NeedCastArrayForTimestamp() /
CastArrayForTimestamp():

  1. Timezone: the Parquet reader always reports LTZ timestamps as UTC, while Paimon exposes
    them in the local timezone.
  2. Unit: the Parquet writer has no SECOND timestamp, so a SECOND column is stored as MILLI and
    cast back to SECOND on read.
  3. Child field names / nullability: a map value field named attrs in the file versus the
    value produced by arrow::map(), a list element field name, or a differing nullable flag.

This PR makes the guard compare only the projection shape:

  • New file-local helper HasSameNestedProjectionShape() in
    src/paimon/format/parquet/parquet_file_batch_reader.cpp:
    • STRUCT: same field count, same field names, recursively same shape (nullability ignored).
    • LIST: compares value_type() only, ignoring the element field name and nullability.
    • MAP: compares key_type() and item_type() only, ignoring the key/value field names.
    • TIMESTAMP leaves: accepted when the units are equal, or when the file is MILLI and the read
      type is SECOND — exactly the conversions ParquetTimestampConverter supports. Timezone
      differences are always accepted and left to the cast path.
    • All other leaves keep the strict Equals() check, and a nested-versus-atomic mismatch is
      rejected.
  • CollectLeafIndices() splits the shared LIST/MAP branch and now recurses through
    ListType::value_type() and MapType::key_type()/item_type() instead of iterating
    file_type->field(i). This also fixes a latent leaf-index bug: arrow::MapType::num_fields()
    is 1 (the entries struct), so the old code fell into the STRUCT branch and matched map children
    by 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) writes
    list<struct<key: utf8, attrs: map<utf8, utf8 ("attrs" value field)>, updated_at: timestamp(us, Asia/Shanghai)>> under a TimezoneGuard, including empty maps, null maps and null timestamps,
    then asserts:
    • the full round trip now succeeds and matches the expected array (previously Status::Invalid);
    • pruning updated_at from the nested struct still fails with
      "Parquet does not support partial projection inside list/map";
    • reading updated_at as utf8 still fails;
    • reading updated_at as timestamp(ns, ...) (an unsupported unit conversion) still fails.

API and Format

No changes.

Documentation

No changes.

Generative AI tooling

No.

@zhf999
zhf999 marked this pull request as draft August 3, 2026 05:24
}

TEST_F(ParquetFileBatchReaderTest, TestNestedListTimestampTimezoneAndMapFieldName) {
const std::string timezone = "Asia/Shanghai";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. nested MILLI -> SECOND unit conversion;
  2. nested MICRO timestamp with a timezone-only difference.

This would verify both the reader-level conversion logic and the complete Parquet write/read path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants