Skip to content

feat(parquet): support vector type storage - #198

Open
ChaomingZhangCN wants to merge 19 commits into
apache:mainfrom
ChaomingZhangCN:codex/vector-parquet-mvp
Open

feat(parquet): support vector type storage#198
ChaomingZhangCN wants to merge 19 commits into
apache:mainfrom
ChaomingZhangCN:codex/vector-parquet-mvp

Conversation

@ChaomingZhangCN

@ChaomingZhangCN ChaomingZhangCN commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Purpose

Linked issue: #197

Related design: PIP-40: Introduce a new Vector data type

This PR implements the first phase of VECTOR<T, N> support in Paimon C++. It introduces the logical type and schema representation, maps it to Arrow FixedSizeList<T, N>, and provides end-to-end Parquet reads and writes while keeping the physical Parquet representation interoperable with standard readers.

The main changes are:

  • Add FieldType::VECTOR and support the SQL-style representation VECTOR<T, N>.

  • Add JSON schema serialization and parsing, for example:

    {
      "type": "VECTOR",
      "element": "FLOAT",
      "length": 3
    }
  • Support BOOLEAN, TINYINT, SMALLINT, INT, BIGINT, FLOAT, and DOUBLE element types.

  • Allow a VECTOR value to be null, but reject null elements inside a non-null VECTOR.

  • Validate that the vector length is positive and that every value matches the dimension declared by the schema.

  • Support VECTOR values nested in STRUCT, LIST, and MAP using the default MAP layout.

  • Preserve nested field IDs and collect nested null-count statistics for VECTOR columns.

  • Reject VECTOR columns in primary, partition, and bucket keys.

  • Restrict VECTOR data files to Parquet in this phase.

  • Reject VECTOR values inside a shared-shredding MAP until the two features can be integrated safely.

Logical and physical representation

flowchart LR
    subgraph WritePath["Write path"]
        W1["Paimon schema VECTOR&lt;T, N&gt;"] --> W2["Arrow logical array FixedSizeList&lt;T, N&gt;"]
        W2 -->|"validate dimension and nulls"| W3["Parquet write array List&lt;T&gt;"]
    end

    subgraph Storage["Physical storage"]
        P[("Parquet data file standard LIST encoding")]
    end

    subgraph ReadPath["Read path"]
        R1["Parquet reader List&lt;T&gt;"] -->|"validate T and N"| R2["Arrow result FixedSizeList&lt;T, N&gt;"]
    end

    W3 --> P --> R1
    E[("Third-party Parquet file ordinary LIST column")] --> R1
Loading

VECTOR has a fixed-size logical representation in Paimon and Arrow, but it is written as a standard Parquet LIST. On read, the converter validates the element type, row length, and nullability before restoring the Arrow FixedSizeList. This also allows a compatible LIST column written by another Parquet implementation to be read as VECTOR when the Paimon schema declares VECTOR<T, N>.

Scope of this phase

flowchart TB
    V["VECTOR&lt;T, N&gt; phase 1"]

    V --> S["Schema and type system"]
    S --> S1["SQL and JSON representation"]
    S --> S2["Arrow FixedSizeList mapping"]
    S --> S3["Dimension and null validation"]

    V --> IO["Parquet I/O"]
    IO --> IO1["FixedSizeList → LIST writes"]
    IO --> IO2["LIST → FixedSizeList reads"]
    IO --> IO3["STRUCT / LIST / MAP nesting"]

    V --> M["Metadata integration"]
    M --> M1["Nested field IDs"]
    M --> M2["Nested null-count statistics"]

    V -.-> F["Follow-up phases"]
    F --> F1["Data Evolution"]
    F --> F2["Dedicated vector-store format"]
    F --> F3["Vector indexes and search"]
    F --> F4["Shared-shredding MAP integration"]
Loading

Data Evolution, a dedicated point-lookup-optimized vector format, vector indexes/search, and shared-shredding MAP integration are intentionally left for follow-up work.

Tests

Added focused coverage for:

  • VECTOR SQL/JSON parsing, serialization, supported element types, and invalid definitions.
  • Arrow type conversion and schema validation.
  • Primary/partition/bucket key restrictions and Parquet-only validation.
  • Nested field ID propagation and nested column statistics.
  • VECTOR-to-LIST and LIST-to-VECTOR conversion, including nullable vectors, sliced arrays, invalid dimensions, and null elements.
  • VECTOR values nested in STRUCT, LIST, and MAP.
  • Reading an ordinary third-party Parquet LIST column as VECTOR.
  • End-to-end Parquet I/O for FLOAT and nested DOUBLE vectors.
  • Table-level append/write/read coverage in WriteAndReadInteTest.TestAppendVector.

Validation performed locally:

  • 22 focused unit tests passed across type/JSON handling, Arrow utilities, schema validation, field IDs/statistics, and Parquet conversion/I/O.
  • paimon-write-and-read-inte-test and paimon-data-evolution-table-test built successfully.
  • All changed files passed the repository pre-commit hooks, including clang-format, C++ lint, CMake format, codespell, and Sphinx lint.
  • git diff --check passed.

The table-level VECTOR test could not be executed successfully in the current macOS Debug build because Arrow is statically embedded in multiple dynamic libraries, causing a cross-DSO RTTI std::bad_cast. The existing non-VECTOR TestAppendSimple baseline also fails in the same environment. The test is included for validation in the supported CI/Linux environment.

API and Format

API: Yes.

  • Add public FieldType::VECTOR = 18.
  • Represent VECTOR columns through Arrow FixedSizeList<T, N> at the C++/Arrow boundary.
  • Extend schema parsing, validation, field type utilities, and nested statistics to recognize VECTOR.

Schema protocol: Yes.

  • Add the VECTOR JSON type with element and length attributes.
  • Add the SQL-style type string VECTOR<T, N>.

Storage format: The change adds VECTOR storage support but does not introduce a new Parquet physical encoding.

  • VECTOR is persisted using the standard Parquet LIST representation.
  • Existing non-VECTOR files and schemas are unchanged.
  • Compatible Parquet LIST data can be restored as VECTOR using the Paimon logical schema.
  • Dedicated vector storage and Data Evolution are outside the scope of this PR.

Documentation

Yes. The data types documentation now describes:

  • VECTOR<T, N> and its Arrow FixedSizeList mapping.
  • Supported element types and nullability rules.
  • The standard Parquet LIST representation used in this phase.
  • Current restrictions and follow-up scope.

Generative AI tooling

Generated-by: OpenAI Codex (GPT-5)

return Status::Invalid(
fmt::format("Vector length must be between 1 and {} (both inclusive), but was {}",
std::numeric_limits<int32_t>::max(), length_token));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we use StringUtils::StringToValue here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

AssignFieldIdsRecursively(vector_type->value_field(),
/*set_field_id=*/false, field_id));
return arrow::field(field->name(),
arrow::fixed_size_list(new_value_field, vector_type->list_size()),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found a blocker.

[Blocking] Reject VECTOR fields in primary-key tables until merge-tree support is implemented

This PR keeps VECTOR columns as Arrow FixedSizeList throughout the framework and converts them to Parquet LIST only at the Parquet writer boundary. Therefore, supporting VECTOR in primary-key tables requires the merge-tree and row-based framework paths to understand FIXED_SIZE_LIST.

The current integration test only covers an append-only table. Primary-key tables containing a VECTOR value column are currently accepted by schema validation but fail in several deterministic places:

  1. InMemorySortBuffer::EstimateMemoryUse has no FIXED_SIZE_LIST branch, so a PK write fails while buffering the first batch.
  2. RowToArrowArrayConverter does not support FixedSizeListBuilder in AppendField, Reserve, or Accumulate. This blocks PK flush, compaction, spill, and PK scan projection.
  3. ColumnarRow, ColumnarRowRef, and ColumnarArray assume GetArray always wraps an Arrow ListArray. They cannot expose values backed by a FixedSizeListArray.

There are also missing branches in additional framework components:

  • InternalRow::CreateFieldGetter does not recognize VECTOR.
  • BinarySerializerUtils::WriteBinaryArray casts the type directly to arrow::ListType.
  • RowCompactedSerializer has no VECTOR reader or writer, affecting lookup persistence.
  • Partial-update and aggregation merge functions create field getters for every value field and therefore cannot be initialized when the schema contains VECTOR.
  • CastedRow also lacks VECTOR handling. This one is not strictly PK-only and may affect schema/stats evolution paths as well.

Most of the above work is specific to primary-key tables and can reasonably be implemented in a follow-up PR. However, this PR must not expose a schema configuration that is known to fail at runtime.

Please update SchemaValidation::ValidateVectorFields to reject any table that both:

  • contains a VECTOR field, including a nested VECTOR field; and
  • has a non-empty primary-key definition.

For example:

if (has_vector && !schema.PrimaryKeys().empty()) {
    return Status::NotImplemented(
        "VECTOR fields in primary-key tables are not implemented yet.");
}

The existing test only verifies that the VECTOR column itself cannot be used as a primary key. Please also add a test for a schema such as:

id BIGINT,
embedding VECTOR<FLOAT, 3>,
PRIMARY KEY (id)

and verify that schema validation rejects it.

The documentation should likewise state that the current implementation supports VECTOR only in append-only Parquet tables. Full primary-key support, including write buffering, row-to-column conversion, reads, compaction, spill, lookup, and merge engines, can then be added and tested in a second PR.

return static_cast<const arrow::FixedSizeListType&>(*type).value_type();
}
return static_cast<const arrow::ListType&>(*type).value_type();
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We recently made some conventions around casts. In the latest code, please use paimon::check_cast instead of static_cast (for more details please refer to docs/code-style.md).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

void ParquetFileBatchReader::SkipLeafIndices(const std::shared_ptr<arrow::DataType>& file_type,
int32_t* leaf_index) {
if (file_type->id() == arrow::Type::STRUCT || file_type->id() == arrow::Type::LIST ||
file_type->id() == arrow::Type::MAP) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I looked into the Rust and Python implementations, and I think we could move ParquetVectorConverter::ConvertToReadType — specifically the list -> fixed size list conversion — up into the framework layer, and introduce a generic reader wrapper on top of ParquetFileBatchReader.

My suggestion would be:

  • convert the schema during SetReadSchema, and
  • convert the data during NextBatch.

You could refer to CompleteRowTrackingFieldsBatchReader for a similar pattern.

This would reduce the amount of change needed in the Parquet plugin layer. Some external engines use their own Parquet plugin implementations, and ideally we want plugin authors to make as few changes as possible when adding vector support.

For the write path, could we just pass fixed size list directly to the Parquet writer? In theory, Parquet does not distinguish between fixed size list and list, and Arrow should be able to handle the conversion properly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For write side, I noticed that the current Paimon library is using Arrow 17, which does not handle fixed size list -> list conversion very well in the presence of nulls. For example, cases like [valid vec, null, valid vec] may fail.

If that’s the case, then the write path may also need to reuse the current ParquetVectorConverter::ConvertToWriteType logic.

Since this issue has already been fixed in newer Arrow versions, I’d suggest keeping the write-side handling as isolated as possible within the Parquet format layer, and adding a TODO so that the extra conversion can be removed directly once we upgrade Arrow.

Comment thread test/inte/write_and_read_inte_test.cpp Outdated
/*is_streaming_mode=*/false));
arrow::Int32Builder ids_builder;
ASSERT_TRUE(ids_builder.AppendValues({1, 2, 3}).ok());
std::shared_ptr<arrow::Array> ids;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For the tests, could you please try to use JSON-style expressions to construct the source or expected arrays, as they’re easier to read and understand.

vector_type.value_field()->WithType(GetPhysicalReadType(vector_type.value_type())));
}
case arrow::Type::STRUCT: {
arrow::FieldVector fields;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thing to note here: for data written by Java, vectors are indeed read back as list, while for data written by Python/Rust, vectors seem to already come back as fixed size list.

So I’d suggest narrowing this conversion to only the case where the file schema is list but the read schema is fixed size list.

Also, could you please add test data generated from both Rust and Java for vector columns, and verify that C++ can read them correctly?

PAIMON_RETURN_NOT_OK_FROM_ARROW(indices_builder.Reserve(index_capacity));
arrow::BooleanBuilder validity_builder(pool);
PAIMON_RETURN_NOT_OK_FROM_ARROW(validity_builder.Reserve(list_array.length()));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’m wondering whether we really need to rebuild the array with a builder here to make a deep copy. It seems Arrow provides arrow::compute::Cast(array, read_type, ...), which may support a more lightweight conversion and avoid deep copying where possible.

PAIMON_RETURN_NOT_OK_FROM_ARROW(offsets_builder.Append(0));

int32_t offset = 0;
for (int64_t i = 0; i < vector_array.length(); ++i) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar to the read path, it seems the write path could also use arrow::cast. We could validate nulls before casting.

@zjw1111 zjw1111 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for implementing VECTOR support here, the LIST-based physical layout matches Paimon Java nicely. One safety concern on the write path.

}
int64_t value_offset = (vector_array->offset() + i) * vector_length;
for (int32_t j = 0; j < vector_length; ++j) {
if (values->IsNull(value_offset + j)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This element scan can read out of bounds on the write path.

AbstractFileStoreWrite::Write builds data via arrow::ImportArray(batch->GetData(), arrow::struct_(write_schema_->fields())), and Arrow's C importer does not verify that a FixedSizeList child holds length * list_size values (ArrayImporter::Visit(const FixedSizeListType&) only checks the child/buffer counts). No Validate() runs before CheckNullabilityMatch either. So if a caller passes an array whose child is shorter than the declared dimension, values->IsNull(value_offset + j) indexes past the child's validity bitmap.

Could you add a structural check before the scan? data->Validate() in AbstractFileStoreWrite::Write would be enough — it is O(1) for this layout and already rejects values->length() < (offset + length) * list_size. It would also turn the opposite case (child longer than declared, currently reinterpreted silently with the wrong dimension) into a clear error instead of corrupt data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

switch (logical_type->id()) {
case arrow::Type::FIXED_SIZE_LIST: {
if (!file_type || file_type->id() != arrow::Type::LIST) {
return logical_type;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

HasSameNestedProjectionShape in parquet_file_batch_reader.cpp does not seem to handle this case yet. Would this cause issues when reading data like List<FixedSizeList<Float32, 3>>?

Also, there seem to be several places in the Parquet code path that do not yet include branches for fixed size list.

Comment thread src/paimon/core/schema/schema_validation.cpp Outdated
Comment thread src/paimon/format/parquet/parquet_format_writer.h Outdated
return false;
}

Status ValidateVectorElements(const std::shared_ptr<arrow::Array>& array) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ValidateVectorElements and ContainsVectorType could go in either arrow_utils or a separate vector_utils, but right now there is too much duplicated implementation.

~ specific language governing permissions and limitations
~ under the License.
-->

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The license for the test data can be omitted.

const auto& entries_type = checked_cast<const arrow::StructType&>(*children[0]->type);
const auto& map_type = checked_cast<const arrow::MapType&>(*read_type);
return std::make_shared<arrow::MapType>(entries_type.field(0), entries_type.field(1),
map_type.keys_sorted());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do both the list and struct branches preserve the original field, while only the MAP branch does not? This drops the original entries field name and metadata.

CreateFileBatchReader(file_format_identifier, data_file_path,
file_meta->file_size, reader_builder));
if (VectorFileBatchReader::ContainsVector(read_schema)) {
file_reader = std::make_unique<VectorFileBatchReader>(std::move(file_reader), pool_);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does the nested-type cast exemption list in field_mapping.cpp (lines 186–192) need to include FIXED_SIZE_LIST as well?

case arrow::Type::LARGE_BINARY:
return RebaseBinaryLike<int64_t>(data, pool);
case arrow::Type::LIST:
case arrow::Type::MAP:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like there is no zero-copy branch for FIXED_SIZE_LIST here. Should we add one? Also, please add an end-to-end test with predicate pushdown to verify that FIXED_SIZE_LIST still works correctly when predicates are pushed down on other fields.

cursoragent and others added 4 commits August 17, 2026 12:10
`ContainsVectorType`, `ContainsVectorField` and `ValidateVectorElements` were
duplicated across schema validation, the Parquet write path and the VECTOR read
wrapper. Move them into `VectorUtils` and let every caller share one
implementation.

While validating a FixedSizeList, also reject a VECTOR whose child does not hold
`length * list_size` values. Arrow does not verify this when importing an array
over the C data interface, so the element scan reading the child validity bitmap
could run past its end on the write path.

Keep the name and metadata of a MAP entries field when rebuilding a nested type
for reads and writes, instead of falling back to Arrow's defaults.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
`NormalizeRecordBatchOffsets` had no FixedSizeList branch, so every sliced VECTOR
column fell back to a full copy of its values. Slice the child instead, the way
the list and struct layouts already do, since a contiguous slice of the parent
always spans a contiguous range of the child.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
A file written by Paimon Rust or Python exposes a VECTOR column as Arrow
FixedSizeList, so the read schema handed to the Parquet plugin keeps that type
too. `HasSameNestedProjectionShape` had no FixedSizeList branch and rejected such
a schema as a partial projection inside list/map, and `CollectLeafIndices` and
`SkipLeafIndices` treated a FixedSizeList as a leaf column.

`FieldMappingBuilder::CreateDataCastExecutors` also has to exempt FixedSizeList
from the scalar cast lookup, like the other nested types, because the reader
reshapes those columns itself.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
Move the `needs_vector_conversion` writer parameter before the memory pool,
build VECTOR test arrays from JSON instead of Arrow builders, and drop the
license header from the VECTOR fixture notes.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
cursoragent and others added 2 commits August 17, 2026 12:34
Reformat the lines added by the previous commits with the clang-format version
pinned in .pre-commit-config.yaml, and pass the reader helper in the Parquet
VECTOR test through an out parameter so it can use the ASSERT_* macros.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
Writing a VECTOR column holding a null value failed with "Lists with non-zero
length null components are not supported": Arrow 17 casts a null FixedSizeList
row to a null LIST slot that still spans `list_size` values, and the Parquet
writer rejects that layout.

Keep the plain cast when a VECTOR has no null value, and otherwise rebuild the
LIST with compacted values so its null slots have a zero length, matching what
Paimon Java writes. Both paths stay inside the Parquet format layer with a TODO
to drop the second one after the Arrow upgrade.

Also cover the table level read path with a predicate pushed down on a
non-vector column next to a VECTOR column.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(result_fields), expected_json)
.ValueOrDie();
ASSERT_TRUE(std::make_shared<arrow::ChunkedArray>(expected)->Equals(actual));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you very much for the contribution and for the patience through multiple rounds of updates. As a final step, could you please add one more end-to-end case that writes and reads nested types with vectors inside struct / map / list?

ASSERT_OK(vector_reader->SetReadSchema(c_read_schema.get(), /*predicate=*/nullptr,
/*selection_bitmap=*/std::nullopt));
ASSERT_OK_AND_ASSIGN(std::shared_ptr<arrow::ChunkedArray> actual,
paimon::test::ReadResultCollector::CollectResult(vector_reader.get()));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not reuse CreateVectorReader here?

if (!schema.PrimaryKeys().empty()) {
return Status::NotImplemented(
"VECTOR fields in primary-key tables are not implemented yet.");
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please explicitly reject Data Evolution mode here.

ASSERT_EQ(id_field->length(), static_cast<int64_t>(expected_ids.size()));
ASSERT_EQ(vector_array->length(), static_cast<int64_t>(expected_vectors.size()));
const int32_t* id_values = id_field->data()->GetValues<int32_t>(1);
for (int64_t i = 0; i < id_field->length(); ++i) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please change this to use ids->Value(i) instead:

auto ids = checked_pointer_cast<arrow::Int32Array>(id_field);
ASSERT_EQ(ids->Value(i), expected_ids[i]);

GetValues<int32_t>(1) already accounts for ArrayData::offset, so adding id_field->offset() again effectively applies the offset twice. This is not exposed at the moment because the current reader happens to produce arrays with offset 0.


TEST_F(ParquetVectorIoTest, ReadJavaFixture) {
ReadFixtureAndCheck(
"java_vector.parquet", arrow::Type::LIST, /*vector_length=*/2,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The helper supports null vectors, but the current Java and Rust fixtures do not contain any null vector, so cross-language compatibility is not fully covered. Please add nullable fixtures for Java LIST and Rust FixedSizeList.

void WriteWithArrowWriter(const std::string& file_path,
const std::shared_ptr<arrow::StructType>& type,
const std::string& json) {
arrow::Result<std::shared_ptr<arrow::Array>> array_result =

@lxy-9602 lxy-9602 Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

WriteWithArrowWriter currently cannot guarantee that the data will be read back as FixedSizeList. If we want to guarantee that arrow::parquet reads it back as FixedSizeList, we need to store the schema.

::parquet::WriterProperties::Builder properties_builder;
auto arrow_properties =
    ::parquet::ArrowWriterProperties::Builder()
        .store_schema()
        ->build();

arrow::Status status = ::parquet::arrow::WriteTable(
    *table,
    arrow_pool_.get(),
    arrow_out,
    /*chunk_size=*/1024,
    properties_builder.build(),
    arrow_properties);

cursoragent and others added 3 commits August 18, 2026 04:22
Data evolution is out of scope for this phase the same way primary-key tables
are, so a schema that enables it together with a VECTOR field must be rejected
rather than fail later at runtime.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
`WriteWithArrowWriter` did not store the Arrow schema, so the file exposed the
column as LIST and `ReadNestedFixedSizeListFile` silently took the LIST to VECTOR
conversion instead of the nested FixedSizeList path it is meant to cover. Store
the schema, assert the layout the file really has, and read a vector nested in a
LIST without null values.

Add the nullable cross-language fixtures: parquet-mr writes the column as a
3-level LIST, which reads back fine, while a writer that stores the Arrow schema
exposes it as FixedSizeList, which Arrow 17 cannot read once a value is null.
Pin both behaviors.

Reuse the shared reader helper in the remaining checks, and read the id and
element values through typed arrays: `GetValues<int32_t>(1)` already accounts for
`ArrayData::offset`, so adding the offset again applied it twice.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
The table level coverage only wrote a top level VECTOR column, so nesting was
exercised at the Parquet layer only.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
CreateVectorReader(file_path, arrow::schema(file_type->fields()), /*predicate=*/nullptr,
/*options=*/{}, /*batch_size=*/10, &reader);
ASSERT_NOK_WITH_MSG(paimon::test::ReadResultCollector::CollectResult(reader.get()),
"Expected all lists to be of size=3");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. For now, since the Arrow fix has landed on master but has not yet been released on the release branch, we plan to keep this PR as-is and not support it for the time being. Later, we can update arrow.patch to temporarily cherry-pick the fix onto the current Arrow 17 we are using, so that Rust-based file can read it correctly.

`GetFileSchema()` is defined in the header and returns
`Result<std::unique_ptr<::ArrowSchema>>`, so instantiating the unique_ptr
destructor needs the complete type. `file_batch_reader.h` only forward declares
it, which broke the clang-tidy run over the header, as the other readers that
define this method inline avoid by including the C data interface too.

Co-authored-by: 小明同学 <ChaomingZhangCN@users.noreply.github.com>
vector_type.ToString()));
}
PAIMON_RETURN_NOT_OK(VectorUtils::ValidateVectorElements(*array));
return array;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P1] Normalize the FixedSizeList output type

This returns the file FixedSizeList type unchanged after comparing only the list size and value data type. The physical child field name/nullability can therefore remain different from the requested logical schema (for example, the Rust fixture uses element: float not null, while the C++ schema uses item: float). Java/C++ LIST files are cast to the logical type, so reading mixed encodings yields unequal batch types and ChunkedArray::Make fails with “Array chunks must all be same type.” Please zero-copy rewrap a copy of array->data() with read_type after validation, and add a regression test that reads mixed LIST/FixedSizeList files using the actual table schema.

A file storing a VECTOR column as FixedSizeList was returned with the
element field of the file, while a file storing it as LIST was cast to
the element field of the read schema. Reading both encodings with one
table schema therefore produced batches of unequal Arrow types, which
cannot be combined into a single result.

Rewrap the validated values in the requested type instead of returning
the file type unchanged.
A reader owns the Arrow memory pool that its batches are allocated
from, so releasing each fixture reader before the collected chunks
crashed while freeing them. Keep both readers alive until the
comparison is done, and iterate over the file names without binding a
reference to a temporary, which -Werror=range-loop-construct rejects.
@lxy-9602

Copy link
Copy Markdown
Collaborator

@dalingmeng Please double-check this pr.

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.

5 participants