Skip to content

Commit 76994af

Browse files
committed
fix: correct partition field handling for non-partitioned tables
- Fix ManifestEntry::operator== logic with proper parentheses grouping - Add null pointer safety in DataFile::Type() for non-partitioned tables - Change partition field type validation from list to struct in manifest reader - Support empty partition structs for non-partitioned tables (n_children == 0) - Update test bounds values to match actual manifest file data This resolves issues with non-partitioned table support and fixes critical bugs in object comparison and type validation.
1 parent 89efa42 commit 76994af

File tree

3 files changed

+16
-12
lines changed

3 files changed

+16
-12
lines changed

src/iceberg/manifest_entry.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,16 @@ namespace iceberg {
2929

3030
bool ManifestEntry::operator==(const ManifestEntry& other) const {
3131
return status == other.status && snapshot_id == other.snapshot_id &&
32-
sequence_number == other.sequence_number &&
33-
file_sequence_number == other.file_sequence_number &&
34-
(data_file && other.data_file && *data_file == *other.data_file) ||
35-
(!data_file && !other.data_file);
32+
sequence_number == other.sequence_number &&
33+
file_sequence_number == other.file_sequence_number &&
34+
((data_file && other.data_file && *data_file == *other.data_file) ||
35+
(!data_file && !other.data_file));
3636
}
3737

3838
std::shared_ptr<StructType> DataFile::Type(std::shared_ptr<StructType> partition_type) {
39+
if (!partition_type) {
40+
partition_type = std::make_shared<StructType>(std::vector<SchemaField>{});
41+
}
3942
return std::make_shared<StructType>(std::vector<SchemaField>{
4043
kContent,
4144
kFilePath,

src/iceberg/manifest_reader_internal.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -368,15 +368,17 @@ Status ParseDataFile(const std::shared_ptr<StructType>& data_file_schema,
368368
break;
369369
case 3: {
370370
if (view_of_file_field->storage_type != ArrowType::NANOARROW_TYPE_STRUCT) {
371-
return InvalidManifest("Field:{} should be a list.", field_name);
371+
return InvalidManifest("Field:{} should be a struct.", field_name);
372372
}
373-
auto view_of_partition = view_of_file_field->children[0];
374-
for (size_t row_idx = 0; row_idx < view_of_partition->length; row_idx++) {
375-
if (ArrowArrayViewIsNull(view_of_partition, row_idx)) {
376-
break;
373+
if (view_of_file_field->n_children > 0) {
374+
auto view_of_partition = view_of_file_field->children[0];
375+
for (size_t row_idx = 0; row_idx < view_of_partition->length; row_idx++) {
376+
if (ArrowArrayViewIsNull(view_of_partition, row_idx)) {
377+
break;
378+
}
379+
ICEBERG_RETURN_UNEXPECTED(
380+
ParseLiteral(view_of_partition, row_idx, manifest_entries));
377381
}
378-
ICEBERG_RETURN_UNEXPECTED(
379-
ParseLiteral(view_of_partition, row_idx, manifest_entries));
380382
}
381383
} break;
382384
case 4:

test/manifest_reader_test.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ class ManifestReaderV2Test : public TempFileTestBase {
142142
std::vector<int64_t> file_sizes = {1344};
143143
std::vector<int64_t> record_counts = {4};
144144

145-
// Real bounds data extracted from the manifest
146145
std::vector<std::map<int32_t, std::vector<uint8_t>>> lower_bounds = {
147146
{{1, {0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
148147
{2, {'r', 'e', 'c', 'o', 'r', 'd', '_', 'f', 'o', 'u', 'r'}},

0 commit comments

Comments
 (0)