SteNicholas commented on code in PR #494:
URL: https://github.com/apache/paimon-cpp/pull/494#discussion_r4444493530
##########
src/paimon/core/manifest/index_manifest_file_handler.cpp:
##########
@@ -255,17 +255,14 @@ IndexManifestFileHandler::SeparateIndexEntries(
Result<std::unique_ptr<IndexManifestFileHandler::IndexManifestFileCombiner>>
IndexManifestFileHandler::GetIndexManifestFileCombine(const std::string&
index_type,
- int32_t bucket_mode) {
+ BucketMode bucket_mode) {
if (index_type != DeletionVectorsIndexFile::DELETION_VECTORS_INDEX &&
index_type != "HASH") {
return std::make_unique<GlobalFileNameCombiner>();
}
- // `bucket_mode` is the configured bucket, not a resolved BucketMode,
standing in for Paimon
- // Java's BucketMode.BUCKET_UNAWARE check. The two agree on every table
that can exist:
- // SchemaValidation rejects the other unaware bucket, 0, and
BucketIdCalculator refuses to
- // write the primary key table on which -1 means HASH_DYNAMIC instead.
Lifting either
- // restriction means passing the resolved BucketMode here, or a dynamic
bucket table would
- // combine its per-bucket deletion vectors by index file name.
- if (index_type == DeletionVectorsIndexFile::DELETION_VECTORS_INDEX &&
bucket_mode == -1) {
+ // A dynamic bucket primary key table also has bucket=-1, but keeps one
deletion vector index
+ // per bucket, so only BUCKET_UNAWARE combines deletion vectors by data
file name.
Review Comment:
Could the comment distinguish the merge key from the consistency check?
`GlobalDeletionVectorCombiner` merges entries by **index file name** and uses
**data file names** to reject duplicate deletion-vector coverage. Saying it
combines by data file name obscures those different roles. Also, "per
(partition, bucket)" would describe the bucketed identity more precisely than
"per bucket".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
