Skip to content

fix: prevent groupByNode panics on empty callback results - #579

Merged
timtofan merged 1 commit into
masterfrom
atimofieiev/group-by-node-empty-input
Oct 21, 2026
Merged

timtofan merged 1 commit into
masterfrom
atimofieiev/group-by-node-empty-input

Conversation

@timtofan

Copy link
Copy Markdown
Collaborator

What issue is this change attempting to solve?

Why

groupByNode and groupByNodes panic when a callback returns a non-nil slice containing zero series. The existing nil check lets that result reach r[0], turning expression evaluation into an index-out-of-range error. The expression evaluator can return this result shape when its context is cancelled.

How does this change solve the problem? Why is this the best approach?

Architecture

This is a local change to expression grouping. Group construction and callback evaluation keep their current boundaries and error-handling behavior. Check the result length before renaming its first series and appending the callback output.

Before / after

Callback result Before After
Nil slice Skip the group Skip the group
Non-nil empty slice Panic at r[0] Skip the group and continue
One or more series Rename the first series to the group key and append the output Same behavior

A non-empty series whose points are all null remains present. This guard operates on the number of returned series, rather than their point values.

How can we be sure this works as expected?

  • The new empty-result regression reproduces the index-out-of-range panic on unchanged master.
  • Regression cases cover both function names, nil and empty callback results, cancellation-shaped results, and a valid group following an empty one. Additional cases cover empty input and preservation of an all-null aggregation.
  • go test ./pkg/expr/functions/groupByNode -count=1 passed.
  • make test passed: 50 packages tested with -tags cairo,nativegzip -race.
  • golangci-lint v2.14.0 passed for the full repository with zero issues.

@timtofan
timtofan force-pushed the atimofieiev/group-by-node-empty-input branch from 3d50b42 to aba1ab6 Compare October 21, 2026 09:12
@timtofan
timtofan merged commit 4fceeb2 into master Oct 21, 2026
6 checks passed
@timtofan
timtofan deleted the atimofieiev/group-by-node-empty-input branch October 21, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants