From 195d4751c5a1021d93b06996b632166fcd955f10 Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Mon, 19 Oct 2026 20:21:06 +0800 Subject: [PATCH 1/2] 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 a1d765d2c8..264517a428 100644 --- a/src/iceberg/arrow_c_data_util.cc +++ b/src/iceberg/arrow_c_data_util.cc @@ -341,8 +341,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 c93a928a3d..5996bc1344 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 @@ -191,4 +192,42 @@ 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)); +} + } // namespace iceberg::internal From 68cffc30ea713297e851f826499cb699780d7db3 Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Mon, 26 Oct 2026 20:10:53 +0800 Subject: [PATCH 2/2] 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/data/file_scan_task_reader.cc | 9 +- src/iceberg/data/file_scan_task_reader.h | 4 + src/iceberg/meson.build | 1 + 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 264517a428..9ce55a22d6 100644 --- a/src/iceberg/arrow_c_data_util.cc +++ b/src/iceberg/arrow_c_data_util.cc @@ -227,7 +227,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; } @@ -253,6 +254,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; } @@ -267,7 +269,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())); @@ -279,6 +285,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; } @@ -308,6 +315,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, @@ -330,27 +341,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); @@ -388,6 +385,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 @@ -397,7 +408,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 iceberg diff --git a/src/iceberg/arrow_c_data_util_internal.h b/src/iceberg/arrow_c_data_util_internal.h index e02db29a64..19a63bd4a2 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 0000000000..e175e72df0 --- /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/data/file_scan_task_reader.cc b/src/iceberg/data/file_scan_task_reader.cc index 7076486ac6..3820deb546 100644 --- a/src/iceberg/data/file_scan_task_reader.cc +++ b/src/iceberg/data/file_scan_task_reader.cc @@ -183,9 +183,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(*data_file, io_, required_schema, task.residual_filter(), name_mapping_, properties_); @@ -206,6 +207,7 @@ class FileScanTaskReader::Impl { projected_schema_(std::move(options.projected_schema)), 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)) {} @@ -214,6 +216,7 @@ class FileScanTaskReader::Impl { std::shared_ptr projected_schema_; 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 a71ef5f84f..fe757589dc 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" @@ -59,6 +60,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/meson.build b/src/iceberg/meson.build index 48b5d42504..b14a847825 100644 --- a/src/iceberg/meson.build +++ b/src/iceberg/meson.build @@ -228,6 +228,7 @@ pkg.generate(iceberg_data_lib) install_headers( [ 'arrow_c_data.h', + 'arrow_validation.h', 'catalog.h', 'constants.h', 'delete_file_index.h', diff --git a/src/iceberg/test/arrow_c_data_util_test.cc b/src/iceberg/test/arrow_c_data_util_test.cc index 5996bc1344..3d2b4a5490 100644 --- a/src/iceberg/test/arrow_c_data_util_test.cc +++ b/src/iceberg/test/arrow_c_data_util_test.cc @@ -68,15 +68,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()); @@ -95,22 +96,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() { @@ -192,42 +185,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); } } // namespace iceberg::internal diff --git a/src/iceberg/test/file_scan_task_reader_test.cc b/src/iceberg/test/file_scan_task_reader_test.cc index 1630a108ab..d2537d9bbd 100644 --- a/src/iceberg/test/file_scan_task_reader_test.cc +++ b/src/iceberg/test/file_scan_task_reader_test.cc @@ -401,6 +401,52 @@ TEST_F(FileScanTaskReaderTest, OpenWithPositionDeletesFiltersRowsAndPrunesPos) { 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,