Skip to content

Commit

Permalink
logicalplan: fix edgecase with scalar() in distribution (#511)
Browse files Browse the repository at this point in the history
scalar() is not really distributive from our experience. This is because
it returns NaN even if the given vector selector doesn't match anything
so as a consequence it is impossible to know whether the NaN is expected
or not from the PoV of the root querier.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
  • Loading branch information
GiedriusS authored Jan 30, 2025
1 parent 761d194 commit ba2290b
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 0 deletions.
6 changes: 6 additions & 0 deletions logicalplan/distribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,12 @@ func isDistributive(expr *Node, skipBinaryPushdown bool, engineLabels map[string
return false
}
}
// scalar() returns NaN if the vector selector returns nothing
// so it's not possible to know which result is correct. Hence,
// it is not distributive.
if e.Func.Name == "scalar" {
return false
}
}

return true
Expand Down
5 changes: 5 additions & 0 deletions logicalplan/distribute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func TestDistributedExecution(t *testing.T) {
expr: `(http_requests_total)`,
expected: `dedup(remote((http_requests_total)), remote((http_requests_total)))`,
},
{
name: "scalar",
expr: `scalar(redis::shard_price_per_month)`,
expected: `scalar(dedup(remote(redis::shard_price_per_month), remote(redis::shard_price_per_month)))`,
},
{
name: "rate",
expr: `rate(http_requests_total[5m])`,
Expand Down

0 comments on commit ba2290b

Please sign in to comment.