From 2862680d327fd378a85fc6c46ccfd833d5124c07 Mon Sep 17 00:00:00 2001 From: Seebs Date: Fri, 20 Jan 2023 13:01:24 -0600 Subject: [PATCH] don't use top-level execute for Count(Distinct()) aggregates This is the only call to top-level execute, as opposed to executeCall, that we make anywhere. I'm trying to clean up this logic and I don't want a special case. Note the change from an empty shard list to specifying the shard list we already had, which I think is logically correct here, but this might actually be a debatable point; it's possible that we really do want to farm that out with an empty shard list, so execute will find one, except that I don't think it *actually* works for us, because the execute is still running on this index, and presumably getting the shard list we started with. Unless this is vulnerable to "shard list can become stale from previous writes". So this passes tests but I'm not 100% sure on the shard decision there. --- executor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor.go b/executor.go index 91aa6d32d..3a7eed8a0 100644 --- a/executor.go +++ b/executor.go @@ -3181,11 +3181,11 @@ func (e *executor) executeGroupBy(ctx context.Context, qcx *Qcx, index string, c } opt.PreTranslated = true - aggregateCount, err := e.execute(ctx, qcx, index, &pql.Query{Calls: []*pql.Call{countDistinctIntersect}}, []uint64{}, opt) + aggregateCount, err := e.executeCall(ctx, qcx, index, countDistinctIntersect, shards, opt) if err != nil { return nil, err } - results[n].Agg = int64(aggregateCount[0].(uint64)) + results[n].Agg = int64(aggregateCount.(uint64)) } }