From e013f7e55f1b634d63a689aa72a2a35dfcdc19e1 Mon Sep 17 00:00:00 2001 From: Noah Held <41909795+zuqq@users.noreply.github.com> Date: Tue, 16 Apr 2024 20:13:15 +0100 Subject: [PATCH] Filter out null values when grouping by annotation (#3521) * Add a regression test Signed-off-by: Noah Held * Fix the bug Signed-off-by: Noah Held * Formatting Signed-off-by: Noah Held --------- Signed-off-by: Noah Held --- internal/lookoutv2/repository/groupjobs_test.go | 7 +++++++ internal/lookoutv2/repository/querybuilder.go | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/internal/lookoutv2/repository/groupjobs_test.go b/internal/lookoutv2/repository/groupjobs_test.go index 328d44650e0..f720c33e530 100644 --- a/internal/lookoutv2/repository/groupjobs_test.go +++ b/internal/lookoutv2/repository/groupjobs_test.go @@ -961,6 +961,13 @@ func TestGroupByAnnotation(t *testing.T) { "test-annotation-1": "test-value-3", }, }, converter, store) + manyJobs(3, &createJobsOpts{ + queue: queue, + jobSet: jobSet, + annotations: map[string]string{ + // Note the absence of the key "test-annotation-1". + }, + }, converter, store) result, err := repo.GroupBy( armadacontext.TODO(), diff --git a/internal/lookoutv2/repository/querybuilder.go b/internal/lookoutv2/repository/querybuilder.go index cfe94cb06cd..a58f43a9b94 100644 --- a/internal/lookoutv2/repository/querybuilder.go +++ b/internal/lookoutv2/repository/querybuilder.go @@ -406,6 +406,17 @@ func (qb *QueryBuilder) GroupByJsonb( return nil, err } + if groupedField.IsAnnotation { + // scanGroup expects values in the grouped-by field not to be null. In + // the case where we are grouping by an annotation key, we end up with + // a `GROUP BY` clause like: + // + // GROUP BY annotations->>'host_instance_id' + // + // This evaluates to null if the annotations object does not contain + // the key in question, so we need to filter out such rows. + filters = append(filters, &model.Filter{Field: groupedField.Field, Match: model.MatchExists, IsAnnotation: true}) + } where, err := qb.makeWhereJsonb(filters) if err != nil { return nil, err