From ec4719cd155d1d58c8aa7c06c7ef24aef6e67622 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Jun 2018 16:18:33 -0400 Subject: [PATCH] Fix partial aggregation for variance(int4) and related aggregates. A typo in numeric_poly_combine caused bogus results for queries using it, but of course would only manifest if parallel aggregation is performed. Reported by Rajkumar Raghuwanshi. David Rowley did the diagnosis and the fix; I editorialized rather heavily on his regression test additions. Back-patch to v10 where the breakage was introduced (by 9cca11c91). Discussion: https://postgr.es/m/CAKcux6nU4E2x8nkSBpLOT2DPvQ5LviJ3SGyAN6Sz7qDH4G4+Pw@mail.gmail.com --- src/backend/utils/adt/numeric.c | 4 ++-- src/test/regress/expected/aggregates.out | 27 ++++++++++++++++++++++++ src/test/regress/sql/aggregates.sql | 18 ++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 074294cbcc..82a14295ee 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -4218,8 +4218,8 @@ numeric_poly_combine(PG_FUNCTION_ARGS) state1->sumX = state2->sumX; state1->sumX2 = state2->sumX2; #else - accum_sum_copy(&state2->sumX, &state1->sumX); - accum_sum_copy(&state2->sumX2, &state1->sumX2); + accum_sum_copy(&state1->sumX, &state2->sumX); + accum_sum_copy(&state1->sumX2, &state2->sumX2); #endif MemoryContextSwitchTo(old_context); diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index f85e913850..10d6bb6824 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -2065,3 +2065,30 @@ SELECT balk(hundred) FROM tenk1; (1 row) ROLLBACK; +-- test coverage for aggregate combine/serial/deserial functions +BEGIN ISOLATION LEVEL REPEATABLE READ; +SET parallel_setup_cost = 0; +SET parallel_tuple_cost = 0; +SET min_parallel_table_scan_size = 0; +SET max_parallel_workers_per_gather = 4; +SET enable_indexonlyscan = off; +-- variance(int4) covers numeric_poly_combine +-- sum(int8) covers int8_avg_combine +EXPLAIN (COSTS OFF) + SELECT variance(unique1::int4), sum(unique1::int8) FROM tenk1; + QUERY PLAN +---------------------------------------------- + Finalize Aggregate + -> Gather + Workers Planned: 4 + -> Partial Aggregate + -> Parallel Seq Scan on tenk1 +(5 rows) + +SELECT variance(unique1::int4), sum(unique1::int8) FROM tenk1; + variance | sum +----------------------+---------- + 8334166.666666666667 | 49995000 +(1 row) + +ROLLBACK; diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 506d0442d7..1b6db50956 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -907,3 +907,21 @@ EXPLAIN (COSTS OFF) SELECT balk(hundred) FROM tenk1; SELECT balk(hundred) FROM tenk1; ROLLBACK; + +-- test coverage for aggregate combine/serial/deserial functions +BEGIN ISOLATION LEVEL REPEATABLE READ; + +SET parallel_setup_cost = 0; +SET parallel_tuple_cost = 0; +SET min_parallel_table_scan_size = 0; +SET max_parallel_workers_per_gather = 4; +SET enable_indexonlyscan = off; + +-- variance(int4) covers numeric_poly_combine +-- sum(int8) covers int8_avg_combine +EXPLAIN (COSTS OFF) + SELECT variance(unique1::int4), sum(unique1::int8) FROM tenk1; + +SELECT variance(unique1::int4), sum(unique1::int8) FROM tenk1; + +ROLLBACK;