You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "wangyong9999 (via GitHub)" <gi...@apache.org> on 2026/08/27 08:31:19 UTC
[PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
wangyong9999 opened a new pull request, #253:
URL: https://github.com/apache/paimon-cpp/pull/253
### Purpose
Linked issue: close #252
A source-backed primary-key index payload uses one immutable ordered source list as its ordinal namespace. After an update retires only part of that list, the current reader requires the payload to equal the complete active source set of its data level. It rejects the still-valid payload and degrades every surviving indexed file to an ordinary scan.
**Key code and data-flow changes**
- Keep the payload's complete source list for group-ordinal localization, but compute coverage from its intersection with eligible active files.
- Accept multiple payload groups at the same level only when their active intersections are disjoint. Reject every group involved in an active-source overlap.
- Map accepted groups back only to files allowed by `PrimaryKeyIndexSourcePolicy::ShouldRead`; uncovered or newly written files retain the existing ordinary-scan fallback.
- Preserve retired prefix/middle source offsets and deletion files when converting surviving files to `IndexedSplit`.
This restores index reuse for surviving compact files without making an uncovered file depend on stale index metadata.
### Tests
- `PkSortedBucketIndexStateTest.*:PrimaryKeySortedIndexScanTest.*`: 39/39 passed on the equivalent x86_64 GCC 8 internal build.
- Full equivalent internal `core_test`: 1849/1849 passed with the repository's documented loopback `no_proxy` environment.
- Added coverage for disjoint same-level payloads, retired prefix/middle ordinal offsets, active overlap rejection, row-count mismatch, a mixed update with deletion-vector preservation, and an active non-eligible APPEND source falling back normally.
### API and Format
No public include API, storage format, or index protocol changes. The change only broadens safe read-side reuse of existing source-backed payload metadata.
### Documentation
Updated the primary-key global-index user guide to describe immutable source groups, partial active coverage, disjoint groups, and fallback boundaries.
### Generative AI tooling
Generated-by: OpenAI Codex (GPT-5)
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#issuecomment-5459597041
Thanks. I narrowed the revision to preserve the existing one-group-per-level lifecycle and added strict `source_meta.DataLevel()` validation; it does not add a writer/protocol state or a C++-only multi-group model. The remaining change fixes C++ read-side validation of an already-written immutable source group after partial retirement. Because no Java behavior or persisted format changes, could this narrow reader correction proceed independently while keeping the Java/C++ lifecycle aligned?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#issuecomment-5460494368
@wangyong9999 I’m not sure this is best described as a bug, since it doesn’t affect correctness and seems more like a performance fallback when the index is not applied. With that in mind, the current title may be slightly misleading.
Also, I think it may be better to wait for the Java side to land first before moving this forward, since the Java implementation covers more complex scenarios and the C++ path is currently a subset of that functionality. From what I can tell, the optimization on the C++ side looks reasonable, but I’d feel more comfortable waiting for full validation on the Java side before treating the overall behavior as fully confirmed. Thanks again for the patience here.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 merged PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on code in PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#discussion_r3877240255
##########
src/paimon/core/index/pksorted/pk_sorted_bucket_index_state.cpp:
##########
@@ -49,10 +59,20 @@ PkSortedBucketIndexState PkSortedBucketIndexState::FromActiveDataFiles(
});
}
- // Match payloads against the expected level sources; anything that does not decode or
- // does not exactly cover its level is rejected.
- std::map<int32_t, std::vector<std::shared_ptr<IndexFileMeta>>> payloads_by_level;
- std::map<int32_t, std::vector<PrimaryKeyIndexSourceMeta>> payload_metas_by_level;
+ std::map<std::string, int64_t> active_sources;
+ std::set<std::string> ambiguous_active_source_names;
+ for (const auto& level_sources : sources_by_level) {
+ for (const PrimaryKeyIndexSourceFile& source : level_sources.second) {
+ if (!active_sources.emplace(source.file_name, source.row_count).second) {
+ ambiguous_active_source_names.insert(source.file_name);
Review Comment:
Is ignoring `source_meta.DataLevel()` intentional to support metadata-only file upgrades? If so, could we add a regression test and document this behavior?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#issuecomment-5460192876
I checked the current Paimon Java and Python readers and confirmed the same exact-active-set rejection there. I filed apache/paimon#9458 and opened the aligned Java/Python fix in apache/paimon#9460. This C++ branch now also binds scan-side source matching to the group's data level, with a regression test for a source moving to another level.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on code in PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#discussion_r3881301625
##########
src/paimon/core/index/pksorted/pk_sorted_bucket_index_state.cpp:
##########
@@ -71,39 +91,86 @@ PkSortedBucketIndexState PkSortedBucketIndexState::FromActiveDataFiles(
continue;
}
PrimaryKeyIndexSourceMeta source_meta = std::move(source_meta_result).value();
- auto desired = sources_by_level.find(source_meta.DataLevel());
- if (desired == sources_by_level.end() || desired->second != source_meta.SourceFiles()) {
+ const std::vector<PrimaryKeyIndexSourceFile>& payload_sources = source_meta.SourceFiles();
+ bool valid_candidate = !payload_sources.empty();
+ std::vector<PrimaryKeyIndexSourceFile> active_intersection;
+ for (size_t i = 0; valid_candidate && i < payload_sources.size(); i++) {
+ const PrimaryKeyIndexSourceFile& source = payload_sources[i];
+ if (i > 0 && payload_sources[i - 1].file_name >= source.file_name) {
+ valid_candidate = false;
+ break;
+ }
+ auto active_source = active_sources.find(source.file_name);
+ if (active_source == active_sources.end()) {
+ continue;
+ }
+ if (active_source->second != source.row_count ||
+ ambiguous_active_source_names.count(source.file_name) > 0) {
+ valid_candidate = false;
+ break;
+ }
+ active_intersection.push_back(source);
+ }
+ if (!valid_candidate || active_intersection.empty()) {
+ rejected.push_back(payload);
+ continue;
+ }
+ std::shared_ptr<PkSortedIndexGroup> group =
+ PkSortedIndexGroup::Create(field_id, index_type, payload_sources, payload, source_meta);
+ if (group == nullptr) {
rejected.push_back(payload);
continue;
}
- payloads_by_level[source_meta.DataLevel()].push_back(payload);
- payload_metas_by_level[source_meta.DataLevel()].push_back(std::move(source_meta));
+ candidates.push_back(
+ {payload, std::move(group), std::move(active_intersection), /*conflicted=*/false});
+ }
+
+ std::map<std::pair<std::string, int64_t>, std::vector<size_t>> candidates_by_source;
Review Comment:
The Java maintainer still keeps exactly one sorted-index group per level and rejects multiple units, so no current writer can produce this state. This conflict graph adds a C++-only lifecycle model beyond the partial-retirement fix. Could we keep the one-group-per-level invariant here and limit this PR to reusing that group’s active intersection?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#issuecomment-5460532445
Agreed. This is a performance fallback, not a correctness issue. I changed the PR title to `perf`, changed both tracking issues to `Improvement`, and clarified that query results remain correct. The aligned Java/Python change is apache/paimon#9460; I will leave this PR pending until that change completes validation and lands, then update here. No further C++ behavior change is planned.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "wangyong9999 (via GitHub)" <gi...@apache.org>.
wangyong9999 commented on code in PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#discussion_r3885097028
##########
src/paimon/core/index/pksorted/pk_sorted_bucket_index_state.cpp:
##########
@@ -71,39 +91,86 @@ PkSortedBucketIndexState PkSortedBucketIndexState::FromActiveDataFiles(
continue;
}
PrimaryKeyIndexSourceMeta source_meta = std::move(source_meta_result).value();
- auto desired = sources_by_level.find(source_meta.DataLevel());
- if (desired == sources_by_level.end() || desired->second != source_meta.SourceFiles()) {
+ const std::vector<PrimaryKeyIndexSourceFile>& payload_sources = source_meta.SourceFiles();
+ bool valid_candidate = !payload_sources.empty();
+ std::vector<PrimaryKeyIndexSourceFile> active_intersection;
+ for (size_t i = 0; valid_candidate && i < payload_sources.size(); i++) {
+ const PrimaryKeyIndexSourceFile& source = payload_sources[i];
+ if (i > 0 && payload_sources[i - 1].file_name >= source.file_name) {
+ valid_candidate = false;
+ break;
+ }
+ auto active_source = active_sources.find(source.file_name);
+ if (active_source == active_sources.end()) {
+ continue;
+ }
+ if (active_source->second != source.row_count ||
+ ambiguous_active_source_names.count(source.file_name) > 0) {
+ valid_candidate = false;
+ break;
+ }
+ active_intersection.push_back(source);
+ }
+ if (!valid_candidate || active_intersection.empty()) {
+ rejected.push_back(payload);
+ continue;
+ }
+ std::shared_ptr<PkSortedIndexGroup> group =
+ PkSortedIndexGroup::Create(field_id, index_type, payload_sources, payload, source_meta);
+ if (group == nullptr) {
rejected.push_back(payload);
continue;
}
- payloads_by_level[source_meta.DataLevel()].push_back(payload);
- payload_metas_by_level[source_meta.DataLevel()].push_back(std::move(source_meta));
+ candidates.push_back(
+ {payload, std::move(group), std::move(active_intersection), /*conflicted=*/false});
+ }
+
+ std::map<std::pair<std::string, int64_t>, std::vector<size_t>> candidates_by_source;
Review Comment:
Updated. The same-level conflict graph and its multi-group test have been removed. The reader again accepts at most one payload group per level and only broadens reuse to that group’s eligible active intersection.
##########
src/paimon/core/index/pksorted/pk_sorted_bucket_index_state.cpp:
##########
@@ -49,10 +59,20 @@ PkSortedBucketIndexState PkSortedBucketIndexState::FromActiveDataFiles(
});
}
- // Match payloads against the expected level sources; anything that does not decode or
- // does not exactly cover its level is rejected.
- std::map<int32_t, std::vector<std::shared_ptr<IndexFileMeta>>> payloads_by_level;
- std::map<int32_t, std::vector<PrimaryKeyIndexSourceMeta>> payload_metas_by_level;
+ std::map<std::string, int64_t> active_sources;
+ std::set<std::string> ambiguous_active_source_names;
+ for (const auto& level_sources : sources_by_level) {
+ for (const PrimaryKeyIndexSourceFile& source : level_sources.second) {
+ if (!active_sources.emplace(source.file_name, source.row_count).second) {
+ ambiguous_active_source_names.insert(source.file_name);
Review Comment:
Not intentional. The current revision now resolves the active set from `source_meta.DataLevel()` and rejects a payload whose declared level has no eligible active intersection. `RejectsPayloadForDifferentActiveDataLevel` covers the metadata-level mismatch.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(index): reuse primary-key index groups with retired source files [paimon-cpp]
Posted by "lxy-9602 (via GitHub)" <gi...@apache.org>.
lxy-9602 commented on PR #253:
URL: https://github.com/apache/paimon-cpp/pull/253#issuecomment-5447902615
The current changes seem reasonable to me. That said, to avoid `PkSortedBucketIndexState` in C++ gradually diverging too much from the Java implementation and becoming harder to align later — for example, if additional features or refactors are introduced on the Java side in the future — I’d suggest merging this logic into `paimon-java` first, and then bringing it into C++.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org