rowexec: fix bug with distinct aggregations on JSONB columns#46711
rowexec: fix bug with distinct aggregations on JSONB columns#46711craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
nit: in the commit message, "a case in for" sounds off.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @rohany)
pkg/sql/rowexec/aggregator.go, line 913 at r1 (raw file):
otherArgTypes []*types.T, ) (bool, error) { // Allocate one EncDatum up front.
nit: to me this comment implies that other EncDatums might be allocated, but they will not. I'd rephrase it to something like "Allocate an EncDatum that will be reused when encoding every argument".
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/rowexec/aggregator.go, line 911 at r1 (raw file):
firstArgType *types.T, otherArgs tree.Datums, otherArgTypes []*types.T,
Another nit: for some reason we seem to be using []types.T everywhere, not []*types.T although for a single we do use *types.T.
Ok, I can update this. Though do I need this? The datums carry the types, I could just use that. |
34ec489 to
0d1710f
Compare
Fixes cockroachdb#46709. A recent change was made to fix encodings to perform aggregations on JSONB and ARRAY columns. That change missed a case in for DISTINCT aggregations, causing a runtime error when executing these queries. Release justification: fixes a bug Release note (bug fix): This PR fixes a bug with distinct aggregations on JSONB columns.
0d1710f to
5849049
Compare
|
bors r=yuzefovich |
Build succeeded |
Fixes #46709.
A recent change (#45229) was made to fix encodings to perform aggregations
on JSONB and ARRAY columns. That change missed a case in for
DISTINCT aggregations, causing a runtime error when executing
these queries.
Release justification: fixes a bug
Release note (bug fix): This PR fixes a bug with distinct aggregations
on JSONB columns.