feat: clustered segments DataSegment changes and PartialLoadMatcher#19462
Conversation
…ialLoadMatcher, remove exact matcher for now
|
if this PR goes in after #19461 it needs adjusted to use the |
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 19 of 19 changed files.
This is an automated review by Codex GPT-5.5
| @Nullable List<String> dimensions, | ||
| @Nullable List<String> metrics, | ||
| @Nullable List<String> projections, | ||
| @Nullable ClusterGroupTuples clusterGroups, |
There was a problem hiding this comment.
[P2] Keep the existing public constructor signature
This inserts clusterGroups into the public constructor that previously took (... projections, ShardSpec, CompactionState, ...). DataSegment is annotated @publicapi, and extension or application code can construct projection-aware segments with that old signature; after this change those callers either fail to compile or, if already compiled, hit NoSuchMethodError even though they do not use cluster groups. Please keep the old overload and delegate with clusterGroups = null instead of replacing the existing public signature.
There was a problem hiding this comment.
fine, I can add it... DataSegment is a bit of a graveyard of deprecated constructors and builder creators though... we need to delete these at some point and just call it out in release notes. I would expect creating DataSegment directly is kind of uncommon in extensions (outside of tests at least).
capistrant
left a comment
There was a problem hiding this comment.
+1 with or without the DataSegment constructor business
FrankChen021
left a comment
There was a problem hiding this comment.
I reviewed the follow-up on the DataSegment constructor compatibility thread and the current incremental update. The previous public constructor overload has been restored and delegates with clusterGroups = null, so the original concern is handled.
Reviewed 4 of 19 changed files.
This is an automated review by Codex GPT-5.5
Description
Companion PR to #19460 that adds adds the
DataSegmentchanges so that clustered segments are visible to coordinators/brokers and aPartialLoadMatcherimplementation that can use wildcards to match cluster groups for partial loads.