Skip to content

Commit 8840174

Browse files
committed
Make schema select fail on missing columns
1 parent 92feae1 commit 8840174

6 files changed

Lines changed: 42 additions & 6 deletions

File tree

src/iceberg/manifest/manifest_reader.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,19 @@ Result<std::shared_ptr<Schema>> ProjectSchema(std::shared_ptr<Schema> schema,
681681
const std::vector<std::string>& columns,
682682
bool case_sensitive) {
683683
if (!columns.empty()) {
684-
return schema->Select(columns, case_sensitive);
684+
if (std::ranges::contains(columns, Schema::kAllColumns)) {
685+
return schema->Select(columns, case_sensitive);
686+
}
687+
688+
std::vector<std::string> existing_columns;
689+
existing_columns.reserve(columns.size());
690+
for (const auto& column : columns) {
691+
ICEBERG_ASSIGN_OR_RAISE(auto field, schema->FindFieldByName(column, case_sensitive));
692+
if (field.has_value()) {
693+
existing_columns.push_back(column);
694+
}
695+
}
696+
return schema->Select(existing_columns, case_sensitive);
685697
}
686698
return schema;
687699
}

src/iceberg/schema.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,10 @@ Result<std::unique_ptr<Schema>> Schema::Select(std::span<const std::string> name
366366
std::unordered_set<int32_t> selected_ids;
367367
for (const auto& name : names) {
368368
ICEBERG_ASSIGN_OR_RAISE(auto result, FindFieldByName(name, case_sensitive));
369-
if (result.has_value()) {
370-
selected_ids.insert(result.value().get().field_id());
369+
if (!result.has_value()) {
370+
return InvalidArgument("Cannot find selected column: {}", name);
371371
}
372+
selected_ids.insert(result.value().get().field_id());
372373
}
373374

374375
PruneColumnVisitor visitor(selected_ids, /*select_full_types=*/true);

src/iceberg/schema.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ class ICEBERG_EXPORT Schema : public StructType {
160160
///
161161
/// \param names Selected field names and nested names are dot-concatenated.
162162
/// \param case_sensitive Whether name matching is case-sensitive (default: true).
163-
/// \return Projected schema containing only selected fields.
163+
/// \return Projected schema containing only selected fields, or an error if any
164+
/// requested field cannot be found.
164165
/// \note If the field name of a nested type has been selected, all of its
165166
/// sub-fields will be selected.
166167
Result<std::unique_ptr<Schema>> Select(std::span<const std::string> names,

src/iceberg/table_scan.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,11 @@ Result<std::unique_ptr<ScanType>> TableScanBuilder<ScanType>::Build() {
413413
ICEBERG_RETURN_UNEXPECTED(context_.Validate());
414414

415415
ICEBERG_ASSIGN_OR_RAISE(auto schema, ResolveSnapshotSchema());
416+
if (!context_.selected_columns.empty()) {
417+
ICEBERG_ASSIGN_OR_RAISE(
418+
std::ignore,
419+
schema.get()->Select(context_.selected_columns, context_.case_sensitive));
420+
}
416421
return ScanType::Make(metadata_, schema.get(), io_, std::move(context_));
417422
}
418423

src/iceberg/test/schema_test.cc

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -816,13 +816,15 @@ INSTANTIATE_TEST_SUITE_P(
816816
.create_schema = []() { return BasicSchema(); },
817817
.select_fields = {"nonexistent"},
818818
.expected_schema = []() { return MakeSchema(); },
819-
.should_succeed = true},
819+
.should_succeed = false,
820+
.expected_error_message = "Cannot find selected column: nonexistent"},
820821

821822
SelectTestParam{.test_name = "SelectCaseSensitive",
822823
.create_schema = []() { return BasicSchema(); },
823824
.select_fields = {"Name"}, // case-sensitive
824825
.expected_schema = []() { return MakeSchema(); },
825-
.should_succeed = true},
826+
.should_succeed = false,
827+
.expected_error_message = "Cannot find selected column: Name"},
826828

827829
SelectTestParam{.test_name = "SelectCaseInsensitive",
828830
.create_schema = []() { return BasicSchema(); },
@@ -840,6 +842,14 @@ INSTANTIATE_TEST_SUITE_P(
840842
.expected_schema = []() { return MakeSchema(Id(), Name()); },
841843
.should_succeed = true},
842844

845+
SelectTestParam{
846+
.test_name = "SelectPartialMissingField",
847+
.create_schema = []() { return AddressSchema(); },
848+
.select_fields = {"id", "typo"},
849+
.expected_schema = []() { return MakeSchema(); },
850+
.should_succeed = false,
851+
.expected_error_message = "Cannot find selected column: typo"},
852+
843853
SelectTestParam{.test_name = "SelectNestedField",
844854
.create_schema = []() { return AddressSchema(); },
845855
.select_fields = {"address.street"},

src/iceberg/test/table_scan_test.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,13 @@ TEST_P(TableScanTest, SchemaWithSelectedColumnsAndFilter) {
677677
}
678678
}
679679

680+
TEST_P(TableScanTest, SelectMissingColumnFails) {
681+
ICEBERG_UNWRAP_OR_FAIL(auto builder,
682+
DataTableScanBuilder::Make(table_metadata_, file_io_));
683+
builder->Select({"id", "typo"});
684+
EXPECT_THAT(builder->Build(), IsError(ErrorKind::kInvalidArgument));
685+
}
686+
680687
INSTANTIATE_TEST_SUITE_P(TableScanVersions, TableScanTest, testing::Values(1, 2, 3));
681688

682689
} // namespace iceberg

0 commit comments

Comments
 (0)