hanahmily commented on code in PR #1132:
URL:
https://github.com/apache/skywalking-banyandb/pull/1132#discussion_r3245953842
##########
pkg/query/logical/measure/measure_plan_distributed.go:
##########
@@ -119,6 +119,7 @@ func newUnresolvedDistributed(query
*measurev1.QueryRequest, pushDownAgg bool) l
}
func (ud *unresolvedDistributed) Analyze(s logical.Schema) (logical.Plan,
error) {
+ indexMode := s.(*schema).measure.IndexMode
Review Comment:
Good catch — confirmed and fixed in 1ac89700.
Verified the panic surface:
- `banyand/dquery/measure.go:58-75` builds a `[]logical.Schema` from
`queryCriteria.Groups`.
- `pkg/query/logical/measure/measure_analyzer.go:213-218` calls
`mergeSchema(ss)` when `len(ss) > 1`.
- `pkg/query/logical/measure/schema.go:186-190` (pre-fix) constructed the
merged `*schema` without setting `measure`.
- My new `s.(*schema).measure.IndexMode` at
`measure_plan_distributed.go:122` would have nil-deref on multi-group queries
regardless of whether the underlying measure was IndexMode.
Went with option (a) — propagate `measure` through `mergeSchema`. All inputs
target the same measure name (different groups), so the underlying
`*databasev1.Measure` is consistent on `IndexMode` by construction; carrying
the first non-nil one is correct. This also retroactively closes the same
latent shape at `measure_plan_indexscan_local.go:106` (not reachable today on
data-node-local queries, but safe-by-construction now).
Added `TestMergeSchema_PropagatesMeasure` in `schema_test.go` covering:
multi-group `IndexMode=true`, multi-group `IndexMode=false`, and the
single-group fast path. All pass.
Not going with option (b) — silently defaulting `IndexMode` to false on
merged schemas would route multi-group IndexMode queries back into the
timestamp-keyed dedup, which is the exact path that lets the cross-node
duplicates this PR is fixing slip through. Better to make the type contract
hold than to special-case the read.
--
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]
