Repository navigation
feat: composite partial load matcher and load spec - #19791
Conversation
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
Reviewed 11 of 11 changed files.
Validation: git diff --check passed.
This is an automated review by Codex GPT-5.6-Sol
| final List<MatchResult> results = new ArrayList<>(matchers.size()); | ||
| for (PartialLoadMatcher matcher : matchers) { | ||
| final MatchResult result = matcher.match(segment, baseLoadSpec); | ||
| if (result == null) { |
There was a problem hiding this comment.
[P1] Preserve valid unions when one projection branch is empty
Projection matchers return null when no configured projection exists, but this code treats that the same as an incompatible matcher and vetoes the entire composite. For example, an older segment lacking one requested projection will full-load under the default FULL_LOAD, even when another member validly selects cluster groups. Empty selections must be distinguished from genuinely incompatible members.
There was a problem hiding this comment.
i was planning to make improvements to the projection matchers which would solve this in a follow-up PR. Projection matchers should basically never return null, and instead if the matching projection is not present it should issue a load-spec that tells things to load the 'base' table projection (since all projections can be computed from the table itself if the projection isn't present).
|
While reviewing this PR I've some thoughts on catalog, partial load match, etc... not feedback on the PR itself, just some future work thoughts:
|
Description
Adds a composite partial load matcher that does a union of matches from a list of child matchers, allowing for example partial loading both cluster groups and projections, or combining matchers to produce matches which would be impossible with a single matcher (like multiple include/exclude patterns which would interfere with each other if within the same wildcard pattern).