Skip to content

Commit 984609c

Browse files
blau-aiclaude
andcommitted
Fix antalya-26.6 build: adapt Iceberg TRUNCATE to 26.6 writer API (#2125)
The #1655 backport was written against the antalya-26.3 Iceberg writer API and did not compile on antalya-26.6, breaking Build (arm_tidy) and Fast test: * Constant.h re-added the `deleted_records` / `deleted_data_files` field aliases that antalya-26.6 already defines, causing macro redefinition errors. Drop the duplicate definitions. * IcebergMetadata::truncate() used the 26.3 FileNamesGenerator / MetadataGenerator / generateManifestList signatures. Rewrite it to follow the antalya-26.6 writer (IcebergStorageSink in IcebergWrites.cpp): - 4-arg FileNamesGenerator over resolver.getTableLocation(); - generateMetadataPathWithInfo() instead of the removed generateMetadataName(); - generateNextMetadata() now returns {snapshot, manifest_list_path}; resolve storage paths via IcebergPathResolver::resolve() and the catalog path via resolveForCatalog(); - generateManifestList(resolver, ...) with empty entry/size vectors. Metadata-only truncate semantics are unchanged. Not built locally; validated by CI on this PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7a63645 commit 984609c

2 files changed

Lines changed: 23 additions & 41 deletions

File tree

src/Storages/ObjectStorage/DataLakes/Iceberg/Constant.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,7 @@ DEFINE_ICEBERG_FIELD_ALIAS(partition_spec, partition-spec);
123123
DEFINE_ICEBERG_FIELD_ALIAS(partition_specs, partition-specs);
124124
DEFINE_ICEBERG_FIELD_ALIAS(spec_id, spec-id);
125125
DEFINE_ICEBERG_FIELD_ALIAS(added_records, added-records);
126-
DEFINE_ICEBERG_FIELD_ALIAS(deleted_records, deleted-records);
127126
DEFINE_ICEBERG_FIELD_ALIAS(added_data_files, added-data-files);
128-
DEFINE_ICEBERG_FIELD_ALIAS(deleted_data_files, deleted-data-files);
129127
DEFINE_ICEBERG_FIELD_ALIAS(added_delete_files, added-delete-files);
130128
DEFINE_ICEBERG_FIELD_ALIAS(added_position_delete_files, added-position-delete-files);
131129
DEFINE_ICEBERG_FIELD_ALIAS(added_position_deletes, added-position-deletes);

src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp

Lines changed: 23 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -635,70 +635,54 @@ void IcebergMetadata::truncate(ContextPtr context, std::shared_ptr<DataLake::ICa
635635
// (distinct from snapshot ID 0 which is a valid snapshot).
636636
Int64 parent_snapshot_id = actual_table_state_snapshot.snapshot_id.value_or(-1);
637637

638-
auto config_path = persistent_components.table_path;
639-
if (!config_path.starts_with('/')) config_path = '/' + config_path;
640-
if (!config_path.ends_with('/')) config_path += "/";
638+
// On antalya-26.6 all metadata paths flow through the table's IcebergPathResolver:
639+
// FileNamesGenerator produces IcebergPathFromMetadata values (relative to the table
640+
// location), and resolver.resolve() / resolveForCatalog() turn those into the storage
641+
// path for I/O and the fully-qualified path the catalog expects. This mirrors the
642+
// write path in IcebergStorageSink (see IcebergWrites.cpp) so transactional (REST) and
643+
// non-transactional catalogs are handled uniformly.
644+
const auto & resolver = persistent_components.path_resolver;
641645

642646
bool is_transactional = (catalog != nullptr && catalog->isTransactional());
643647

644-
// Transactional catalogs (e.g. REST) require a fully-qualified blob URI
645-
// (scheme://bucket/path) so the catalog can resolve the metadata location
646-
// independently of any local path configuration. Non-transactional catalogs
647-
// use bare paths relative to the object storage root.
648-
FileNamesGenerator filename_generator;
649-
if (is_transactional || context->getSettingsRef()[Setting::write_full_path_in_iceberg_metadata])
650-
{
651-
String location = metadata_object->getValue<String>(Iceberg::f_location);
652-
if (!location.ends_with("/")) location += "/";
653-
filename_generator = FileNamesGenerator(
654-
location, config_path, is_transactional,
655-
persistent_components.metadata_compression_method, write_format);
656-
}
657-
else
658-
{
659-
filename_generator = FileNamesGenerator(
660-
config_path, config_path, false,
661-
persistent_components.metadata_compression_method, write_format);
662-
}
648+
FileNamesGenerator filename_generator(
649+
resolver.getTableLocation(),
650+
is_transactional,
651+
persistent_components.metadata_compression_method,
652+
write_format);
663653

664654
Int32 new_metadata_version = actual_table_state_snapshot.metadata_version + 1;
665655
filename_generator.setVersion(new_metadata_version);
666656

667-
auto [metadata_name, storage_metadata_name] = filename_generator.generateMetadataName();
657+
auto metadata_info = filename_generator.generateMetadataPathWithInfo();
668658

669-
auto [new_snapshot, manifest_list_name, storage_manifest_list_name] = MetadataGenerator(metadata_object).generateNextMetadata(
670-
filename_generator, metadata_name, parent_snapshot_id,
659+
auto [new_snapshot, manifest_list_path] = MetadataGenerator(metadata_object).generateNextMetadata(
660+
filename_generator, metadata_info.path, parent_snapshot_id,
671661
/* added_files */ 0, /* added_records */ 0, /* added_files_size */ 0,
672662
/* num_partitions */ 0, /* added_delete_files */ 0, /* num_deleted_rows */ 0,
673663
std::nullopt, std::nullopt, /*is_truncate=*/true);
674664

665+
auto storage_manifest_list_name = resolver.resolve(manifest_list_path);
666+
675667
auto write_settings = context->getWriteSettings();
676668
auto buf = object_storage->writeObject(
677669
StoredObject(storage_manifest_list_name),
678670
WriteMode::Rewrite, std::nullopt,
679671
DBMS_DEFAULT_BUFFER_SIZE, write_settings);
680672

681-
generateManifestList(filename_generator, metadata_object, object_storage,
682-
context, {}, new_snapshot, 0, *buf, Iceberg::FileContentType::DATA, /*use_previous_snapshots=*/false);
673+
// Truncate writes a metadata-only overwrite snapshot: an empty manifest list
674+
// (no manifest entries, no sizes) that supersedes all previous snapshots.
675+
generateManifestList(resolver, metadata_object, object_storage,
676+
context, {}, new_snapshot, {}, *buf, Iceberg::FileContentType::DATA, /*use_previous_snapshots=*/false);
683677
buf->finalize();
684678

685679
String metadata_content = dumpMetadataObjectToString(metadata_object);
686-
writeMessageToFile(metadata_content, storage_metadata_name, object_storage,
680+
writeMessageToFile(metadata_content, resolver.resolve(metadata_info.path), object_storage,
687681
context, "*", "", persistent_components.metadata_compression_method);
688682

689683
if (catalog)
690684
{
691-
// Transactional catalogs require a fully-qualified blob URI so the catalog
692-
// can resolve the metadata location independently of local path configuration.
693-
String catalog_filename = metadata_name;
694-
if (is_transactional)
695-
{
696-
// Build full URI from the table's location field (e.g. "s3://bucket/namespace.table")
697-
// combined with the relative metadata name.
698-
String location = metadata_object->getValue<String>(Iceberg::f_location);
699-
if (!location.ends_with("/")) location += "/";
700-
catalog_filename = location + metadata_name;
701-
}
685+
String catalog_filename = resolver.resolveForCatalog(metadata_info.path);
702686

703687
const auto & [namespace_name, table_name] = DataLake::parseTableName(storage_id.getTableName());
704688
if (!catalog->updateMetadata(namespace_name, table_name, catalog_filename, new_snapshot))

0 commit comments

Comments
 (0)