From eb492a63195d8c17e59f7bdda44cecc43cd53b8f Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Mon, 19 Oct 2026 20:21:06 +0800 Subject: [PATCH 1/3] fix(arrow): use FULL nanoarrow validation in ProjectBatch ProjectBatchNanoarrow validated incoming batches at NANOARROW_VALIDATION_LEVEL_DEFAULT before using ArrowArrayViewGet{String,Bytes}Unsafe and raw list/map offset arithmetic per row. nanoarrow documents these Unsafe accessors as memory-safe only after FULL validation: DEFAULT only checks the first/last offsets, not that every intermediate offset is non-decreasing and in-bounds. A non-monotonic offset can produce a negative size that overflows to a huge size_t in the append path, or an out-of-range offset that gets copied into the output batch. Raise the validation level to FULL, matching what manifest_reader.cc already does for the same untrusted-batch boundary. --- src/iceberg/arrow_c_data_util.cc | 8 ++++- src/iceberg/test/arrow_c_data_util_test.cc | 39 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/iceberg/arrow_c_data_util.cc b/src/iceberg/arrow_c_data_util.cc index eb7e90b71..73c8c8eeb 100644 --- a/src/iceberg/arrow_c_data_util.cc +++ b/src/iceberg/arrow_c_data_util.cc @@ -352,8 +352,14 @@ Result ProjectBatchNanoarrow( internal::ArrowArrayViewGuard input_view_guard(&input_view); ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( ArrowArrayViewSetArray(&input_view, &input_batch, &error), error); + // FULL validation is required here: the append path below uses + // ArrowArrayViewGet{String,Bytes}Unsafe and raw list/map offset arithmetic, + // which nanoarrow only documents as memory-safe after FULL validation + // (DEFAULT does not check that every offsets-buffer entry is non-decreasing + // and in-bounds). This matches the FULL validation already performed on the + // equivalent untrusted-batch boundary in manifest_reader.cc. ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( - ArrowArrayViewValidate(&input_view, NANOARROW_VALIDATION_LEVEL_DEFAULT, &error), + ArrowArrayViewValidate(&input_view, NANOARROW_VALIDATION_LEVEL_FULL, &error), error); ArrowArray output_array; diff --git a/src/iceberg/test/arrow_c_data_util_test.cc b/src/iceberg/test/arrow_c_data_util_test.cc index 35bb30fdb..b48c5bba1 100644 --- a/src/iceberg/test/arrow_c_data_util_test.cc +++ b/src/iceberg/test/arrow_c_data_util_test.cc @@ -20,6 +20,7 @@ /// \file arrow_c_data_util_test.cc /// Verifies ProjectBatch behavior across registered implementations. +#include #include #include #include @@ -231,6 +232,44 @@ TEST(ProjectBatchTest, ProjectionRejectsNestedPruning) { EXPECT_THAT(projection, IsError(ErrorKind::kInvalidArgument)); } +TEST(ProjectBatchTest, RejectsNonMonotonicStringOffsets) { + auto schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "value", string())}); + + // Arrow's builders always emit monotonic offsets, so a batch with a + // corrupt offsets buffer has to be hand-assembled via the nanoarrow C ABI. + // offsets = [0, 5, 2] with a 2-byte data buffer: DEFAULT validation only + // checks the first (0) and last (2) offsets, both of which are consistent + // with the 2-byte buffer, so it accepts this array. FULL validation also + // walks the offsets buffer and rejects it because 2 < 5. + ArrowArray root; + ASSERT_EQ(ArrowArrayInitFromType(&root, NANOARROW_TYPE_STRUCT), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAllocateChildren(&root, 1), NANOARROW_OK); + + ArrowArray* value_array = root.children[0]; + ASSERT_EQ(ArrowArrayInitFromType(value_array, NANOARROW_TYPE_STRING), NANOARROW_OK); + const std::array raw_offsets = {0, 5, 2}; + ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 1), raw_offsets.data(), + raw_offsets.size() * sizeof(int32_t)), + NANOARROW_OK); + ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 2), "ab", 2), NANOARROW_OK); + value_array->length = 2; + value_array->null_count = 0; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(value_array, nullptr), NANOARROW_OK); + + root.length = 2; + root.null_count = 0; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(&root, nullptr), NANOARROW_OK); + + auto projection = ProjectionContext::Make(*schema, *schema, nullptr); + ASSERT_THAT(projection, IsOk()); + + // No rows are selected, so the malformed offsets must be caught by batch + // validation itself rather than by appending the corrupt row. + auto result = ProjectBatch(&root, /*row_indices=*/{}, projection.value()); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); +} + TEST(WriteProjectionTest, IdenticalSchemasTransferBatchWithoutConversion) { auto input_schema = MakeFullSchema(); auto write_schema = MakeFullSchema(); From 2f40e33d7ae64c5bf09bb5f933fa126066625cfe Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Mon, 26 Oct 2026 20:10:53 +0800 Subject: [PATCH 2/3] fix(arrow): make projection validation configurable Keep full validation as the default across projection backends. Allow callers to select basic validation for trusted arrays through scan reader options. --- src/iceberg/arrow_c_data_util.cc | 57 ++++--- src/iceberg/arrow_c_data_util_internal.h | 22 ++- src/iceberg/arrow_validation.h | 35 +++++ src/iceberg/cpp20_compatibility_internal.h | 1 + src/iceberg/data/file_scan_task_reader.cc | 9 +- src/iceberg/data/file_scan_task_reader.h | 4 + src/iceberg/test/arrow_c_data_util_test.cc | 146 +++++++++++------- .../test/file_scan_task_reader_test.cc | 46 ++++++ 8 files changed, 232 insertions(+), 88 deletions(-) create mode 100644 src/iceberg/arrow_validation.h diff --git a/src/iceberg/arrow_c_data_util.cc b/src/iceberg/arrow_c_data_util.cc index 73c8c8eeb..a239ddef4 100644 --- a/src/iceberg/arrow_c_data_util.cc +++ b/src/iceberg/arrow_c_data_util.cc @@ -238,7 +238,8 @@ ProjectionContext::ProjectionContext(ProjectionContext&& other) noexcept input_arrow_schema_(other.input_arrow_schema_), output_arrow_schema_(other.output_arrow_schema_), project_batch_function_(std::exchange(other.project_batch_function_, nullptr)), - project_batch_state_(std::move(other.project_batch_state_)) { + project_batch_state_(std::move(other.project_batch_state_)), + validation_mode_(other.validation_mode_) { other.input_arrow_schema_.release = nullptr; other.output_arrow_schema_.release = nullptr; } @@ -264,6 +265,7 @@ ProjectionContext& ProjectionContext::operator=(ProjectionContext&& other) noexc other.output_arrow_schema_.release = nullptr; project_batch_function_ = std::exchange(other.project_batch_function_, nullptr); project_batch_state_ = std::move(other.project_batch_state_); + validation_mode_ = other.validation_mode_; return *this; } @@ -278,7 +280,11 @@ ProjectionContext::~ProjectionContext() { Result ProjectionContext::Make( const Schema& input_schema, const Schema& output_schema, - ProjectionContext::ProjectBatchFunction project_batch_function) { + ProjectionContext::ProjectBatchFunction project_batch_function, + ArrowValidationMode validation_mode) { + ICEBERG_PRECHECK(validation_mode == ArrowValidationMode::kFull || + validation_mode == ArrowValidationMode::kBasic, + "Invalid Arrow validation mode"); ICEBERG_ASSIGN_OR_RAISE( auto selected_field_indices, BuildSelectedFieldIndices(input_schema.fields(), output_schema.fields())); @@ -290,6 +296,7 @@ Result ProjectionContext::Make( ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(input_schema, &context.input_arrow_schema_)); ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(output_schema, &context.output_arrow_schema_)); context.project_batch_function_ = project_batch_function; + context.validation_mode_ = validation_mode; return context; } @@ -319,6 +326,10 @@ ProjectionContext::ProjectBatchState& ProjectionContext::project_batch_state() { return project_batch_state_; } +ArrowValidationMode ProjectionContext::validation_mode() const { + return validation_mode_; +} + void ProjectionContext::RegisterProjectBatchFunction( ProjectionContext::ProjectBatchFunction project_batch_function) { ICEBERG_DCHECK(project_batch_function != nullptr, @@ -341,27 +352,13 @@ auto ProjectionContext::ResolveProjectBatchFunction() namespace { -Result ProjectBatchNanoarrow( - const ArrowSchema& input_arrow_schema, const ArrowArray& input_batch, - std::span row_indices, const ArrowSchema& output_arrow_schema, - std::span selected_field_indices) { - ArrowArrayView input_view; +Result ProjectBatchNanoarrow(const ArrowSchema& input_arrow_schema, + const ArrowArray& input_batch, + std::span row_indices, + const ArrowSchema& output_arrow_schema, + std::span selected_field_indices, + const ArrowArrayView& input_view) { ArrowError error; - ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( - ArrowArrayViewInitFromSchema(&input_view, &input_arrow_schema, &error), error); - internal::ArrowArrayViewGuard input_view_guard(&input_view); - ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( - ArrowArrayViewSetArray(&input_view, &input_batch, &error), error); - // FULL validation is required here: the append path below uses - // ArrowArrayViewGet{String,Bytes}Unsafe and raw list/map offset arithmetic, - // which nanoarrow only documents as memory-safe after FULL validation - // (DEFAULT does not check that every offsets-buffer entry is non-decreasing - // and in-bounds). This matches the FULL validation already performed on the - // equivalent untrusted-batch boundary in manifest_reader.cc. - ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( - ArrowArrayViewValidate(&input_view, NANOARROW_VALIDATION_LEVEL_FULL, &error), - error); - ArrowArray output_array; ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( ArrowArrayInitFromSchema(&output_array, &output_arrow_schema, &error), error); @@ -399,6 +396,20 @@ Result ProjectBatch(ArrowArray* input_batch, ICEBERG_PRECHECK(input_batch != nullptr, "input_batch must not be null"); internal::ArrowArrayGuard input_batch_guard(input_batch); + ArrowArrayView input_view; + ArrowError error; + ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( + ArrowArrayViewInitFromSchema(&input_view, &projection.input_arrow_schema(), &error), + error); + internal::ArrowArrayViewGuard input_view_guard(&input_view); + ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( + ArrowArrayViewSetArray(&input_view, input_batch, &error), error); + if (projection.validation_mode() == ArrowValidationMode::kFull) { + ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( + ArrowArrayViewValidate(&input_view, NANOARROW_VALIDATION_LEVEL_FULL, &error), + error); + } + auto project_batch_function = projection.project_batch_function(); if (project_batch_function != nullptr) { // ProjectBatch owns input_batch. Arrow-backed implementations import it and clear @@ -408,7 +419,7 @@ Result ProjectBatch(ArrowArray* input_batch, return ProjectBatchNanoarrow(projection.input_arrow_schema(), *input_batch, row_indices, projection.output_arrow_schema(), - projection.selected_field_indices()); + projection.selected_field_indices(), input_view); } namespace { diff --git a/src/iceberg/arrow_c_data_util_internal.h b/src/iceberg/arrow_c_data_util_internal.h index 899695b6c..525792291 100644 --- a/src/iceberg/arrow_c_data_util_internal.h +++ b/src/iceberg/arrow_c_data_util_internal.h @@ -31,6 +31,7 @@ #include #include "iceberg/arrow_c_data.h" +#include "iceberg/arrow_validation.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -68,13 +69,16 @@ class ICEBERG_EXPORT ProjectionContext { /// \param project_batch_function Optional implementation returned by /// ProjectionContext::ResolveProjectBatchFunction, or nullptr to use the nanoarrow /// path. + /// \param validation_mode Full content validation by default; Basic requires trusted + /// input. /// \note It validates that output_schema selects or reorders complete top-level fields /// by field id. Nested pruning and type changes are rejected. The input_schema and /// output_schema passed to Make must outlive the context. ProjectBatch may lazily /// initialize backend cache; do not share one context across concurrent calls. - static Result Make(const Schema& input_schema, - const Schema& output_schema, - ProjectBatchFunction project_batch_function); + static Result Make( + const Schema& input_schema, const Schema& output_schema, + ProjectBatchFunction project_batch_function, + ArrowValidationMode validation_mode = ArrowValidationMode::kFull); ProjectionContext(ProjectionContext&&) noexcept; ProjectionContext& operator=(ProjectionContext&&) noexcept; @@ -97,6 +101,8 @@ class ICEBERG_EXPORT ProjectionContext { ProjectBatchState& project_batch_state(); + ArrowValidationMode validation_mode() const; + private: ProjectionContext() = default; @@ -109,6 +115,7 @@ class ICEBERG_EXPORT ProjectionContext { ArrowSchema output_arrow_schema_{}; ProjectBatchFunction project_batch_function_ = nullptr; ProjectBatchState project_batch_state_; + ArrowValidationMode validation_mode_ = ArrowValidationMode::kFull; }; /// \brief Concept for sources that can be wrapped as ArrowArrayStreams. @@ -228,10 +235,11 @@ ICEBERG_EXPORT Result MakeArrowArrayStream( /// \brief Project selected rows from a batch into complete top-level fields. /// /// `input_batch` is consumed by this function. If the projection carries a registered -/// implementation, the call is delegated to it; otherwise the built-in nanoarrow -/// implementation is used. The projection must have been created for the stable schema -/// pair that describes the input batch and requested output. This function does not -/// revalidate schema compatibility on each batch. +/// implementation, the call is delegated to it after validation; otherwise the built-in +/// nanoarrow implementation is used. The projection must have been created for the stable +/// schema pair that describes the input batch and requested output. This function does +/// not revalidate schema compatibility on each batch. Input arrays are validated +/// according to the context policy before either backend runs. /// /// \param input_batch Owned Arrow C Data batch to project. /// \param row_indices Zero-based row positions to copy from `input_batch`. diff --git a/src/iceberg/arrow_validation.h b/src/iceberg/arrow_validation.h new file mode 100644 index 000000000..e175e72df --- /dev/null +++ b/src/iceberg/arrow_validation.h @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include + +namespace iceberg { + +/// Validation policy for Arrow arrays before accessing their contents. +enum class ArrowValidationMode : uint8_t { + /// Validate structure and contents, including intermediate offsets. + kFull, + /// Validate structure without scanning all contents. The caller must guarantee + /// that the array is valid and remains unchanged while it is being accessed. + kBasic, +}; + +} // namespace iceberg diff --git a/src/iceberg/cpp20_compatibility_internal.h b/src/iceberg/cpp20_compatibility_internal.h index 12114d8fe..3a59694f3 100644 --- a/src/iceberg/cpp20_compatibility_internal.h +++ b/src/iceberg/cpp20_compatibility_internal.h @@ -25,6 +25,7 @@ #pragma once #include "iceberg/arrow_c_data.h" +#include "iceberg/arrow_validation.h" #include "iceberg/catalog.h" #include "iceberg/catalog/catalog_util.h" #include "iceberg/catalog/memory/in_memory_catalog.h" diff --git a/src/iceberg/data/file_scan_task_reader.cc b/src/iceberg/data/file_scan_task_reader.cc index bf23c5c36..d5ab7e843 100644 --- a/src/iceberg/data/file_scan_task_reader.cc +++ b/src/iceberg/data/file_scan_task_reader.cc @@ -213,9 +213,10 @@ class FileScanTaskReader::Impl { auto required_schema = delete_filter->RequiredSchema(); auto project_batch_function = ProjectionContext::ResolveProjectBatchFunction(); // ProjectionContext borrows schemas that are kept in MergeOnReadStreamSource. - ICEBERG_ASSIGN_OR_RAISE(auto projection_context, - ProjectionContext::Make(*required_schema, *projected_schema_, - project_batch_function)); + ICEBERG_ASSIGN_OR_RAISE( + auto projection_context, + ProjectionContext::Make(*required_schema, *projected_schema_, + project_batch_function, arrow_validation_mode_)); auto options = MakeReaderOptions(task, io_, required_schema, filter, case_sensitive_, name_mapping_, properties_, data_file->first_row_id, @@ -238,6 +239,7 @@ class FileScanTaskReader::Impl { case_sensitive_(options.case_sensitive), name_mapping_(std::move(options.name_mapping)), properties_(ReaderProperties::FromMap(options.properties)), + arrow_validation_mode_(options.arrow_validation_mode), field_lookup_(std::move(field_lookup)), delete_counter_(std::move(delete_counter)) {} @@ -247,6 +249,7 @@ class FileScanTaskReader::Impl { bool case_sensitive_; std::shared_ptr name_mapping_; ReaderProperties properties_; + ArrowValidationMode arrow_validation_mode_; DeleteFilter::FieldLookup field_lookup_; std::shared_ptr delete_counter_; }; diff --git a/src/iceberg/data/file_scan_task_reader.h b/src/iceberg/data/file_scan_task_reader.h index 591d49c92..12c37a416 100644 --- a/src/iceberg/data/file_scan_task_reader.h +++ b/src/iceberg/data/file_scan_task_reader.h @@ -28,6 +28,7 @@ #include #include "iceberg/arrow_c_data.h" +#include "iceberg/arrow_validation.h" #include "iceberg/iceberg_data_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -61,6 +62,9 @@ class ICEBERG_DATA_EXPORT FileScanTaskReader { std::shared_ptr name_mapping; /// Format-specific or implementation-specific options for data readers. std::unordered_map properties; + /// Validation before merge-on-read batch projection. Basic requires trusted arrays. + /// This does not change validation in readers or delete filtering. + ArrowValidationMode arrow_validation_mode = ArrowValidationMode::kFull; }; /// \brief Create a reusable task reader from shared read context. diff --git a/src/iceberg/test/arrow_c_data_util_test.cc b/src/iceberg/test/arrow_c_data_util_test.cc index b48c5bba1..08d8736cd 100644 --- a/src/iceberg/test/arrow_c_data_util_test.cc +++ b/src/iceberg/test/arrow_c_data_util_test.cc @@ -77,15 +77,16 @@ ProjectionContext::ProjectBatchFunction ArrowComputeFunction() { std::shared_ptr<::arrow::RecordBatch> RunProjectBatch( const ::arrow::RecordBatch& batch, const std::vector& alive_indices, const Schema& required_schema, const Schema& projected_schema, - ProjectionContext::ProjectBatchFunction project_batch_function) { + ProjectionContext::ProjectBatchFunction project_batch_function, + ArrowValidationMode validation_mode) { ArrowSchema c_schema; ArrowArray c_array; EXPECT_TRUE(::arrow::ExportRecordBatch(batch, &c_array, &c_schema).ok()); ArrowSchemaGuard schema_guard(&c_schema); ArrowArrayGuard array_guard(&c_array); - auto projection = - ProjectionContext::Make(required_schema, projected_schema, project_batch_function); + auto projection = ProjectionContext::Make(required_schema, projected_schema, + project_batch_function, validation_mode); EXPECT_THAT(projection, IsOk()); auto result = ProjectBatch(&c_array, alive_indices, projection.value()); @@ -104,22 +105,14 @@ void ExpectProjectBatch(const ::arrow::RecordBatch& batch, const Schema& required_schema, const Schema& projected_schema, std::string_view expected_json) { auto expected = MakeBatch(projected_schema, expected_json); - auto nanoarrow = - RunProjectBatch(batch, alive_indices, required_schema, projected_schema, nullptr); - auto arrow_compute = RunProjectBatch(batch, alive_indices, required_schema, - projected_schema, ArrowComputeFunction()); - - EXPECT_TRUE(nanoarrow->Equals(*expected)) << "nanoarrow:\n" - << nanoarrow->ToString() << "expected:\n" - << expected->ToString(); - EXPECT_TRUE(arrow_compute->Equals(*expected)) - << "arrow_compute:\n" - << arrow_compute->ToString() << "expected:\n" - << expected->ToString(); - EXPECT_TRUE(nanoarrow->Equals(*arrow_compute)) - << "nanoarrow:\n" - << nanoarrow->ToString() << "arrow_compute:\n" - << arrow_compute->ToString(); + for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { + auto nanoarrow = RunProjectBatch(batch, alive_indices, required_schema, + projected_schema, nullptr, mode); + auto arrow_compute = RunProjectBatch(batch, alive_indices, required_schema, + projected_schema, ArrowComputeFunction(), mode); + EXPECT_TRUE(nanoarrow->Equals(*expected)) << nanoarrow->ToString(); + EXPECT_TRUE(arrow_compute->Equals(*expected)) << arrow_compute->ToString(); + } } std::shared_ptr MakeFullSchema() { @@ -232,42 +225,85 @@ TEST(ProjectBatchTest, ProjectionRejectsNestedPruning) { EXPECT_THAT(projection, IsError(ErrorKind::kInvalidArgument)); } -TEST(ProjectBatchTest, RejectsNonMonotonicStringOffsets) { - auto schema = std::make_shared( - std::vector{SchemaField::MakeRequired(1, "value", string())}); - - // Arrow's builders always emit monotonic offsets, so a batch with a - // corrupt offsets buffer has to be hand-assembled via the nanoarrow C ABI. - // offsets = [0, 5, 2] with a 2-byte data buffer: DEFAULT validation only - // checks the first (0) and last (2) offsets, both of which are consistent - // with the 2-byte buffer, so it accepts this array. FULL validation also - // walks the offsets buffer and rejects it because 2 < 5. - ArrowArray root; - ASSERT_EQ(ArrowArrayInitFromType(&root, NANOARROW_TYPE_STRUCT), NANOARROW_OK); - ASSERT_EQ(ArrowArrayAllocateChildren(&root, 1), NANOARROW_OK); - - ArrowArray* value_array = root.children[0]; - ASSERT_EQ(ArrowArrayInitFromType(value_array, NANOARROW_TYPE_STRING), NANOARROW_OK); - const std::array raw_offsets = {0, 5, 2}; - ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 1), raw_offsets.data(), - raw_offsets.size() * sizeof(int32_t)), - NANOARROW_OK); - ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 2), "ab", 2), NANOARROW_OK); - value_array->length = 2; - value_array->null_count = 0; - ASSERT_EQ(ArrowArrayFinishBuildingDefault(value_array, nullptr), NANOARROW_OK); - - root.length = 2; - root.null_count = 0; - ASSERT_EQ(ArrowArrayFinishBuildingDefault(&root, nullptr), NANOARROW_OK); - - auto projection = ProjectionContext::Make(*schema, *schema, nullptr); - ASSERT_THAT(projection, IsOk()); - - // No rows are selected, so the malformed offsets must be caught by batch - // validation itself rather than by appending the corrupt row. - auto result = ProjectBatch(&root, /*row_indices=*/{}, projection.value()); - EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); +TEST(ProjectBatchTest, ValidationModesWithNonMonotonicStringOffsets) { + for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { + for (auto function : + {ProjectionContext::ProjectBatchFunction{nullptr}, ArrowComputeFunction()}) { + auto schema = std::make_shared( + std::vector{SchemaField::MakeRequired(1, "value", string())}); + + // DEFAULT accepts the first/last offsets; FULL rejects the intermediate decrease. + ArrowArray root; + ASSERT_EQ(ArrowArrayInitFromType(&root, NANOARROW_TYPE_STRUCT), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAllocateChildren(&root, 1), NANOARROW_OK); + + ArrowArray* value_array = root.children[0]; + ASSERT_EQ(ArrowArrayInitFromType(value_array, NANOARROW_TYPE_STRING), NANOARROW_OK); + const std::array raw_offsets = {0, 5, 2}; + ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 1), raw_offsets.data(), + raw_offsets.size() * sizeof(int32_t)), + NANOARROW_OK); + ASSERT_EQ(ArrowBufferAppend(ArrowArrayBuffer(value_array, 2), "ab", 2), + NANOARROW_OK); + value_array->length = 2; + value_array->null_count = 0; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(value_array, nullptr), NANOARROW_OK); + + root.length = 2; + root.null_count = 0; + ASSERT_EQ(ArrowArrayFinishBuildingDefault(&root, nullptr), NANOARROW_OK); + + auto projection = mode == ArrowValidationMode::kFull + ? ProjectionContext::Make(*schema, *schema, function) + : ProjectionContext::Make(*schema, *schema, function, mode); + ASSERT_THAT(projection, IsOk()); + + // Empty selection lets Basic skip the content check without reading corrupt data. + auto result = ProjectBatch(&root, /*row_indices=*/{}, projection.value()); + if (mode == ArrowValidationMode::kFull) { + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); + EXPECT_THAT(result, HasErrorMessage("element size >= 0")); + } else { + EXPECT_THAT(result, IsOk()); + } + if (result.has_value()) { + ArrowArrayGuard guard(&result.value()); + EXPECT_EQ(result->length, 0); + } + EXPECT_EQ(root.release, nullptr); + } + } +} + +TEST(ProjectBatchTest, BasicStillRejectsInvalidStructure) { + auto schema = MakeFullSchema(); + auto batch = MakeBatch(*schema, R"([[1,"a",1.0]])"); + for (auto function : + {ProjectionContext::ProjectBatchFunction{nullptr}, ArrowComputeFunction()}) { + ArrowArray array; + ASSERT_TRUE(::arrow::ExportRecordBatch(*batch, &array).ok()); + array.length = -1; + ICEBERG_UNWRAP_OR_FAIL( + auto projection, + ProjectionContext::Make(*schema, *schema, function, ArrowValidationMode::kBasic)); + auto result = ProjectBatch(&array, {}, projection); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); + EXPECT_THAT(result, HasErrorMessage("length")); + EXPECT_EQ(array.release, nullptr); + } +} + +TEST(ProjectBatchTest, MovePreservesValidationMode) { + auto schema = MakeFullSchema(); + ICEBERG_UNWRAP_OR_FAIL( + auto basic, + ProjectionContext::Make(*schema, *schema, nullptr, ArrowValidationMode::kBasic)); + auto moved = std::move(basic); + EXPECT_EQ(moved.validation_mode(), ArrowValidationMode::kBasic); + ICEBERG_UNWRAP_OR_FAIL(auto full, ProjectionContext::Make(*schema, *schema, nullptr)); + EXPECT_EQ(full.validation_mode(), ArrowValidationMode::kFull); + full = std::move(moved); + EXPECT_EQ(full.validation_mode(), ArrowValidationMode::kBasic); } TEST(WriteProjectionTest, IdenticalSchemasTransferBatchWithoutConversion) { diff --git a/src/iceberg/test/file_scan_task_reader_test.cc b/src/iceberg/test/file_scan_task_reader_test.cc index 8b0d79d15..b907e1b97 100644 --- a/src/iceberg/test/file_scan_task_reader_test.cc +++ b/src/iceberg/test/file_scan_task_reader_test.cc @@ -671,6 +671,52 @@ TEST_F(FileScanTaskReaderTest, OpenWithDeletionVectorFiltersRows) { ASSERT_NO_FATAL_FAILURE(VerifyStream(&stream, R"([[1, "Foo"], [3, "Baz"]])")); } +TEST_F(FileScanTaskReaderTest, PassesValidationModeToProjection) { + ICEBERG_UNWRAP_OR_FAIL( + auto data_file, + MakeDataFile(table_schema_, + R"([[1, "Foo", "blue"], [2, "Bar", "red"], [3, "Baz", "green"]])")); + ICEBERG_UNWRAP_OR_FAIL( + auto pos_delete, MakePositionDeleteFile(CreateNewTempFilePathWithSuffix(".parquet"), + {1}, data_file->file_path)); + FileScanTask task(data_file, {pos_delete}); + + struct RestoreProjection { + ProjectionContext::ProjectBatchFunction function = + ProjectionContext::ResolveProjectBatchFunction(); + ~RestoreProjection() { ProjectionContext::RegisterProjectBatchFunction(function); } + } restore; + ProjectionContext::RegisterProjectBatchFunction( + [](ArrowArray*, std::span, + ProjectionContext& projection) -> Result { + return InvalidArgument("projection mode: {}", + projection.validation_mode() == ArrowValidationMode::kBasic + ? "basic" + : "full"); + }); + + for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { + FileScanTaskReader::Options options{ + .io = file_io_, + .table_schema = table_schema_, + .projected_schema = projected_schema_, + }; + if (mode == ArrowValidationMode::kBasic) { + options.arrow_validation_mode = mode; + } + ICEBERG_UNWRAP_OR_FAIL(auto reader, FileScanTaskReader::Make(std::move(options))); + ICEBERG_UNWRAP_OR_FAIL(auto stream, reader->Open(task)); + + ArrowArray array{}; + EXPECT_NE(stream.get_next(&stream, &array), 0); + EXPECT_THAT(stream.get_last_error(&stream), + ::testing::HasSubstr(mode == ArrowValidationMode::kBasic + ? "projection mode: basic" + : "projection mode: full")); + stream.release(&stream); + } +} + TEST_F(FileScanTaskReaderTest, OpenWithEqualityDeletesAddsAndPrunesDeleteOnlyColumns) { ICEBERG_UNWRAP_OR_FAIL( auto data_file, From 03e146e5d5b88fd648d36b6154ea89c38d843cb5 Mon Sep 17 00:00:00 2001 From: Gang Wu Date: Fri, 30 Oct 2026 16:46:28 +0800 Subject: [PATCH 3/3] simplify it --- src/iceberg/arrow_c_data_util.cc | 18 +++---- src/iceberg/arrow_c_data_util_internal.h | 17 +++---- src/iceberg/arrow_validation.h | 35 ------------- src/iceberg/cpp20_compatibility_internal.h | 1 - src/iceberg/data/file_scan_task_reader.cc | 5 +- src/iceberg/data/file_scan_task_reader.h | 4 -- src/iceberg/file_reader.h | 3 ++ src/iceberg/test/arrow_c_data_util_test.cc | 50 +++++++++---------- .../test/file_scan_task_reader_test.cc | 17 +++---- 9 files changed, 51 insertions(+), 99 deletions(-) delete mode 100644 src/iceberg/arrow_validation.h diff --git a/src/iceberg/arrow_c_data_util.cc b/src/iceberg/arrow_c_data_util.cc index a239ddef4..915eeabcc 100644 --- a/src/iceberg/arrow_c_data_util.cc +++ b/src/iceberg/arrow_c_data_util.cc @@ -239,7 +239,7 @@ ProjectionContext::ProjectionContext(ProjectionContext&& other) noexcept output_arrow_schema_(other.output_arrow_schema_), project_batch_function_(std::exchange(other.project_batch_function_, nullptr)), project_batch_state_(std::move(other.project_batch_state_)), - validation_mode_(other.validation_mode_) { + validate_batch_(other.validate_batch_) { other.input_arrow_schema_.release = nullptr; other.output_arrow_schema_.release = nullptr; } @@ -265,7 +265,7 @@ ProjectionContext& ProjectionContext::operator=(ProjectionContext&& other) noexc other.output_arrow_schema_.release = nullptr; project_batch_function_ = std::exchange(other.project_batch_function_, nullptr); project_batch_state_ = std::move(other.project_batch_state_); - validation_mode_ = other.validation_mode_; + validate_batch_ = other.validate_batch_; return *this; } @@ -280,11 +280,7 @@ ProjectionContext::~ProjectionContext() { Result ProjectionContext::Make( const Schema& input_schema, const Schema& output_schema, - ProjectionContext::ProjectBatchFunction project_batch_function, - ArrowValidationMode validation_mode) { - ICEBERG_PRECHECK(validation_mode == ArrowValidationMode::kFull || - validation_mode == ArrowValidationMode::kBasic, - "Invalid Arrow validation mode"); + ProjectionContext::ProjectBatchFunction project_batch_function, bool validate_batch) { ICEBERG_ASSIGN_OR_RAISE( auto selected_field_indices, BuildSelectedFieldIndices(input_schema.fields(), output_schema.fields())); @@ -296,7 +292,7 @@ Result ProjectionContext::Make( ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(input_schema, &context.input_arrow_schema_)); ICEBERG_RETURN_UNEXPECTED(ToArrowSchema(output_schema, &context.output_arrow_schema_)); context.project_batch_function_ = project_batch_function; - context.validation_mode_ = validation_mode; + context.validate_batch_ = validate_batch; return context; } @@ -326,9 +322,7 @@ ProjectionContext::ProjectBatchState& ProjectionContext::project_batch_state() { return project_batch_state_; } -ArrowValidationMode ProjectionContext::validation_mode() const { - return validation_mode_; -} +bool ProjectionContext::validate_batch() const { return validate_batch_; } void ProjectionContext::RegisterProjectBatchFunction( ProjectionContext::ProjectBatchFunction project_batch_function) { @@ -404,7 +398,7 @@ Result ProjectBatch(ArrowArray* input_batch, internal::ArrowArrayViewGuard input_view_guard(&input_view); ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( ArrowArrayViewSetArray(&input_view, input_batch, &error), error); - if (projection.validation_mode() == ArrowValidationMode::kFull) { + if (projection.validate_batch()) { ICEBERG_NANOARROW_RETURN_UNEXPECTED_WITH_ERROR( ArrowArrayViewValidate(&input_view, NANOARROW_VALIDATION_LEVEL_FULL, &error), error); diff --git a/src/iceberg/arrow_c_data_util_internal.h b/src/iceberg/arrow_c_data_util_internal.h index 525792291..c5d203973 100644 --- a/src/iceberg/arrow_c_data_util_internal.h +++ b/src/iceberg/arrow_c_data_util_internal.h @@ -31,7 +31,6 @@ #include #include "iceberg/arrow_c_data.h" -#include "iceberg/arrow_validation.h" #include "iceberg/iceberg_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -69,16 +68,16 @@ class ICEBERG_EXPORT ProjectionContext { /// \param project_batch_function Optional implementation returned by /// ProjectionContext::ResolveProjectBatchFunction, or nullptr to use the nanoarrow /// path. - /// \param validation_mode Full content validation by default; Basic requires trusted - /// input. + /// \param validate_batch Whether to perform full content validation. Disable only for + /// trusted input. /// \note It validates that output_schema selects or reorders complete top-level fields /// by field id. Nested pruning and type changes are rejected. The input_schema and /// output_schema passed to Make must outlive the context. ProjectBatch may lazily /// initialize backend cache; do not share one context across concurrent calls. - static Result Make( - const Schema& input_schema, const Schema& output_schema, - ProjectBatchFunction project_batch_function, - ArrowValidationMode validation_mode = ArrowValidationMode::kFull); + static Result Make(const Schema& input_schema, + const Schema& output_schema, + ProjectBatchFunction project_batch_function, + bool validate_batch = true); ProjectionContext(ProjectionContext&&) noexcept; ProjectionContext& operator=(ProjectionContext&&) noexcept; @@ -101,7 +100,7 @@ class ICEBERG_EXPORT ProjectionContext { ProjectBatchState& project_batch_state(); - ArrowValidationMode validation_mode() const; + bool validate_batch() const; private: ProjectionContext() = default; @@ -115,7 +114,7 @@ class ICEBERG_EXPORT ProjectionContext { ArrowSchema output_arrow_schema_{}; ProjectBatchFunction project_batch_function_ = nullptr; ProjectBatchState project_batch_state_; - ArrowValidationMode validation_mode_ = ArrowValidationMode::kFull; + bool validate_batch_ = true; }; /// \brief Concept for sources that can be wrapped as ArrowArrayStreams. diff --git a/src/iceberg/arrow_validation.h b/src/iceberg/arrow_validation.h deleted file mode 100644 index e175e72df..000000000 --- a/src/iceberg/arrow_validation.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -#pragma once - -#include - -namespace iceberg { - -/// Validation policy for Arrow arrays before accessing their contents. -enum class ArrowValidationMode : uint8_t { - /// Validate structure and contents, including intermediate offsets. - kFull, - /// Validate structure without scanning all contents. The caller must guarantee - /// that the array is valid and remains unchanged while it is being accessed. - kBasic, -}; - -} // namespace iceberg diff --git a/src/iceberg/cpp20_compatibility_internal.h b/src/iceberg/cpp20_compatibility_internal.h index 3a59694f3..12114d8fe 100644 --- a/src/iceberg/cpp20_compatibility_internal.h +++ b/src/iceberg/cpp20_compatibility_internal.h @@ -25,7 +25,6 @@ #pragma once #include "iceberg/arrow_c_data.h" -#include "iceberg/arrow_validation.h" #include "iceberg/catalog.h" #include "iceberg/catalog/catalog_util.h" #include "iceberg/catalog/memory/in_memory_catalog.h" diff --git a/src/iceberg/data/file_scan_task_reader.cc b/src/iceberg/data/file_scan_task_reader.cc index d5ab7e843..8d3c73281 100644 --- a/src/iceberg/data/file_scan_task_reader.cc +++ b/src/iceberg/data/file_scan_task_reader.cc @@ -216,7 +216,8 @@ class FileScanTaskReader::Impl { ICEBERG_ASSIGN_OR_RAISE( auto projection_context, ProjectionContext::Make(*required_schema, *projected_schema_, - project_batch_function, arrow_validation_mode_)); + project_batch_function, + properties_.Get(ReaderProperties::kArrowValidateBatch))); auto options = MakeReaderOptions(task, io_, required_schema, filter, case_sensitive_, name_mapping_, properties_, data_file->first_row_id, @@ -239,7 +240,6 @@ class FileScanTaskReader::Impl { case_sensitive_(options.case_sensitive), name_mapping_(std::move(options.name_mapping)), properties_(ReaderProperties::FromMap(options.properties)), - arrow_validation_mode_(options.arrow_validation_mode), field_lookup_(std::move(field_lookup)), delete_counter_(std::move(delete_counter)) {} @@ -249,7 +249,6 @@ class FileScanTaskReader::Impl { bool case_sensitive_; std::shared_ptr name_mapping_; ReaderProperties properties_; - ArrowValidationMode arrow_validation_mode_; DeleteFilter::FieldLookup field_lookup_; std::shared_ptr delete_counter_; }; diff --git a/src/iceberg/data/file_scan_task_reader.h b/src/iceberg/data/file_scan_task_reader.h index 12c37a416..591d49c92 100644 --- a/src/iceberg/data/file_scan_task_reader.h +++ b/src/iceberg/data/file_scan_task_reader.h @@ -28,7 +28,6 @@ #include #include "iceberg/arrow_c_data.h" -#include "iceberg/arrow_validation.h" #include "iceberg/iceberg_data_export.h" #include "iceberg/result.h" #include "iceberg/type_fwd.h" @@ -62,9 +61,6 @@ class ICEBERG_DATA_EXPORT FileScanTaskReader { std::shared_ptr name_mapping; /// Format-specific or implementation-specific options for data readers. std::unordered_map properties; - /// Validation before merge-on-read batch projection. Basic requires trusted arrays. - /// This does not change validation in readers or delete filtering. - ArrowValidationMode arrow_validation_mode = ArrowValidationMode::kFull; }; /// \brief Create a reusable task reader from shared read context. diff --git a/src/iceberg/file_reader.h b/src/iceberg/file_reader.h index 13e8b3ad2..eded9191c 100644 --- a/src/iceberg/file_reader.h +++ b/src/iceberg/file_reader.h @@ -80,6 +80,9 @@ class ICEBERG_EXPORT ReaderProperties : public ConfigBase { /// Only the Parquet reader honors this option; other readers ignore it. /// Default: false (use 32-bit offset list). inline static Entry kArrowUseLargeList{"read.arrow.use-large-list", false}; + /// \brief Validate Arrow batches before merge-on-read projection. + /// Default: true. Disable only when the reader is trusted to produce valid arrays. + inline static Entry kArrowValidateBatch{"read.arrow.validate-batch", true}; /// \brief Use footer statistics to prune Parquet row groups. inline static Entry kParquetRowGroupFilter{ "read.parquet.row-group-filter.enabled", true}; diff --git a/src/iceberg/test/arrow_c_data_util_test.cc b/src/iceberg/test/arrow_c_data_util_test.cc index 08d8736cd..a21f88c43 100644 --- a/src/iceberg/test/arrow_c_data_util_test.cc +++ b/src/iceberg/test/arrow_c_data_util_test.cc @@ -77,8 +77,7 @@ ProjectionContext::ProjectBatchFunction ArrowComputeFunction() { std::shared_ptr<::arrow::RecordBatch> RunProjectBatch( const ::arrow::RecordBatch& batch, const std::vector& alive_indices, const Schema& required_schema, const Schema& projected_schema, - ProjectionContext::ProjectBatchFunction project_batch_function, - ArrowValidationMode validation_mode) { + ProjectionContext::ProjectBatchFunction project_batch_function, bool validate_batch) { ArrowSchema c_schema; ArrowArray c_array; EXPECT_TRUE(::arrow::ExportRecordBatch(batch, &c_array, &c_schema).ok()); @@ -86,7 +85,7 @@ std::shared_ptr<::arrow::RecordBatch> RunProjectBatch( ArrowArrayGuard array_guard(&c_array); auto projection = ProjectionContext::Make(required_schema, projected_schema, - project_batch_function, validation_mode); + project_batch_function, validate_batch); EXPECT_THAT(projection, IsOk()); auto result = ProjectBatch(&c_array, alive_indices, projection.value()); @@ -105,11 +104,12 @@ void ExpectProjectBatch(const ::arrow::RecordBatch& batch, const Schema& required_schema, const Schema& projected_schema, std::string_view expected_json) { auto expected = MakeBatch(projected_schema, expected_json); - for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { + for (bool validate_batch : {true, false}) { auto nanoarrow = RunProjectBatch(batch, alive_indices, required_schema, - projected_schema, nullptr, mode); - auto arrow_compute = RunProjectBatch(batch, alive_indices, required_schema, - projected_schema, ArrowComputeFunction(), mode); + projected_schema, nullptr, validate_batch); + auto arrow_compute = + RunProjectBatch(batch, alive_indices, required_schema, projected_schema, + ArrowComputeFunction(), validate_batch); EXPECT_TRUE(nanoarrow->Equals(*expected)) << nanoarrow->ToString(); EXPECT_TRUE(arrow_compute->Equals(*expected)) << arrow_compute->ToString(); } @@ -225,8 +225,8 @@ TEST(ProjectBatchTest, ProjectionRejectsNestedPruning) { EXPECT_THAT(projection, IsError(ErrorKind::kInvalidArgument)); } -TEST(ProjectBatchTest, ValidationModesWithNonMonotonicStringOffsets) { - for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { +TEST(ProjectBatchTest, ValidateBatchControlsNonMonotonicStringOffsets) { + for (bool validate_batch : {true, false}) { for (auto function : {ProjectionContext::ProjectBatchFunction{nullptr}, ArrowComputeFunction()}) { auto schema = std::make_shared( @@ -253,14 +253,13 @@ TEST(ProjectBatchTest, ValidationModesWithNonMonotonicStringOffsets) { root.null_count = 0; ASSERT_EQ(ArrowArrayFinishBuildingDefault(&root, nullptr), NANOARROW_OK); - auto projection = mode == ArrowValidationMode::kFull - ? ProjectionContext::Make(*schema, *schema, function) - : ProjectionContext::Make(*schema, *schema, function, mode); + auto projection = + ProjectionContext::Make(*schema, *schema, function, validate_batch); ASSERT_THAT(projection, IsOk()); - // Empty selection lets Basic skip the content check without reading corrupt data. + // Empty selection skips the content check without reading corrupt data. auto result = ProjectBatch(&root, /*row_indices=*/{}, projection.value()); - if (mode == ArrowValidationMode::kFull) { + if (validate_batch) { EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); EXPECT_THAT(result, HasErrorMessage("element size >= 0")); } else { @@ -275,7 +274,7 @@ TEST(ProjectBatchTest, ValidationModesWithNonMonotonicStringOffsets) { } } -TEST(ProjectBatchTest, BasicStillRejectsInvalidStructure) { +TEST(ProjectBatchTest, SkippedContentValidationStillRejectsInvalidStructure) { auto schema = MakeFullSchema(); auto batch = MakeBatch(*schema, R"([[1,"a",1.0]])"); for (auto function : @@ -285,7 +284,7 @@ TEST(ProjectBatchTest, BasicStillRejectsInvalidStructure) { array.length = -1; ICEBERG_UNWRAP_OR_FAIL( auto projection, - ProjectionContext::Make(*schema, *schema, function, ArrowValidationMode::kBasic)); + ProjectionContext::Make(*schema, *schema, function, /*validate_batch=*/false)); auto result = ProjectBatch(&array, {}, projection); EXPECT_THAT(result, IsError(ErrorKind::kInvalidArrowData)); EXPECT_THAT(result, HasErrorMessage("length")); @@ -293,17 +292,18 @@ TEST(ProjectBatchTest, BasicStillRejectsInvalidStructure) { } } -TEST(ProjectBatchTest, MovePreservesValidationMode) { +TEST(ProjectBatchTest, MovePreservesValidateBatch) { auto schema = MakeFullSchema(); ICEBERG_UNWRAP_OR_FAIL( - auto basic, - ProjectionContext::Make(*schema, *schema, nullptr, ArrowValidationMode::kBasic)); - auto moved = std::move(basic); - EXPECT_EQ(moved.validation_mode(), ArrowValidationMode::kBasic); - ICEBERG_UNWRAP_OR_FAIL(auto full, ProjectionContext::Make(*schema, *schema, nullptr)); - EXPECT_EQ(full.validation_mode(), ArrowValidationMode::kFull); - full = std::move(moved); - EXPECT_EQ(full.validation_mode(), ArrowValidationMode::kBasic); + auto without_validation, + ProjectionContext::Make(*schema, *schema, nullptr, /*validate_batch=*/false)); + auto moved = std::move(without_validation); + EXPECT_FALSE(moved.validate_batch()); + ICEBERG_UNWRAP_OR_FAIL(auto validation, + ProjectionContext::Make(*schema, *schema, nullptr)); + EXPECT_TRUE(validation.validate_batch()); + validation = std::move(moved); + EXPECT_FALSE(validation.validate_batch()); } TEST(WriteProjectionTest, IdenticalSchemasTransferBatchWithoutConversion) { diff --git a/src/iceberg/test/file_scan_task_reader_test.cc b/src/iceberg/test/file_scan_task_reader_test.cc index b907e1b97..596ec488a 100644 --- a/src/iceberg/test/file_scan_task_reader_test.cc +++ b/src/iceberg/test/file_scan_task_reader_test.cc @@ -671,7 +671,7 @@ TEST_F(FileScanTaskReaderTest, OpenWithDeletionVectorFiltersRows) { ASSERT_NO_FATAL_FAILURE(VerifyStream(&stream, R"([[1, "Foo"], [3, "Baz"]])")); } -TEST_F(FileScanTaskReaderTest, PassesValidationModeToProjection) { +TEST_F(FileScanTaskReaderTest, ReadsArrowValidateBatchProperty) { ICEBERG_UNWRAP_OR_FAIL( auto data_file, MakeDataFile(table_schema_, @@ -690,19 +690,17 @@ TEST_F(FileScanTaskReaderTest, PassesValidationModeToProjection) { [](ArrowArray*, std::span, ProjectionContext& projection) -> Result { return InvalidArgument("projection mode: {}", - projection.validation_mode() == ArrowValidationMode::kBasic - ? "basic" - : "full"); + projection.validate_batch() ? "full" : "basic"); }); - for (auto mode : {ArrowValidationMode::kFull, ArrowValidationMode::kBasic}) { + for (bool validate_batch : {true, false}) { FileScanTaskReader::Options options{ .io = file_io_, .table_schema = table_schema_, .projected_schema = projected_schema_, }; - if (mode == ArrowValidationMode::kBasic) { - options.arrow_validation_mode = mode; + if (!validate_batch) { + options.properties[ReaderProperties::kArrowValidateBatch.key()] = "false"; } ICEBERG_UNWRAP_OR_FAIL(auto reader, FileScanTaskReader::Make(std::move(options))); ICEBERG_UNWRAP_OR_FAIL(auto stream, reader->Open(task)); @@ -710,9 +708,8 @@ TEST_F(FileScanTaskReaderTest, PassesValidationModeToProjection) { ArrowArray array{}; EXPECT_NE(stream.get_next(&stream, &array), 0); EXPECT_THAT(stream.get_last_error(&stream), - ::testing::HasSubstr(mode == ArrowValidationMode::kBasic - ? "projection mode: basic" - : "projection mode: full")); + ::testing::HasSubstr(validate_batch ? "projection mode: full" + : "projection mode: basic")); stream.release(&stream); } }