From 232019b79229db7a438f00f358d86659fbd6fcb9 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Wed, 3 Jul 2019 23:46:06 +1200 Subject: [PATCH] Don't remove surplus columns from GROUP BY for inheritance parents d4c3a156c added code to remove columns that were not part of a table's PRIMARY KEY constraint from the GROUP BY clause when all the primary key columns were present in the group by. This is fine to do since we know that there will only be one row per group coming from this relation. However, the logic failed to consider inheritance parent relations. These can have child relations without a primary key, but even if they did, they could duplicate one of the parent's rows or one from another child relation. In this case, those additional GROUP BY columns are required. Fix this by disabling the optimization for inheritance parent tables. In v11 and beyond, partitioned tables are fine since partitions cannot overlap and before v11 partitioned tables could not have a primary key. Reported-by: Manuel Rigger Discussion: http://postgr.es/m/CA+u7OA7VLKf_vEr6kLF3MnWSA9LToJYncgpNX2tQ-oWzYCBQAw@mail.gmail.com Backpatch-through: 9.6 --- src/backend/optimizer/plan/planner.c | 8 ++++++++ src/test/regress/expected/aggregates.out | 24 +++++++++++++++++++++++- src/test/regress/sql/aggregates.sql | 10 +++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index f6047804904..ab84a347b39 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -2849,6 +2849,14 @@ remove_useless_groupby_columns(PlannerInfo *root) if (rte->rtekind != RTE_RELATION) continue; + /* + * We must skip inheritance parent tables as some of the child rels + * may cause duplicate rows. We can skip partitioned tables too since + * these cannot have a primary key. + */ + if (rte->inh) + continue; + /* Nothing to do unless this rel has multiple Vars in GROUP BY */ relattnos = groupbyattnos[relid]; if (bms_membership(relattnos) != BMS_MULTIPLE) diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 4d97a841aea..9c1281de24f 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1017,7 +1017,29 @@ explain (costs off) select * from t3 group by a,b,c; -> Seq Scan on t3 (3 rows) -drop table t1; +create temp table t1c () inherits (t1); +-- Ensure we don't remove any columns when t1 has a child table +explain (costs off) select * from t1 group by a,b,c,d; + QUERY PLAN +------------------------------------- + HashAggregate + Group Key: t1.a, t1.b, t1.c, t1.d + -> Append + -> Seq Scan on t1 + -> Seq Scan on t1c +(5 rows) + +-- Okay to remove columns if we're only querying the parent. +explain (costs off) select * from only t1 group by a,b,c,d; + QUERY PLAN +---------------------- + HashAggregate + Group Key: a, b + -> Seq Scan on t1 +(3 rows) + +drop table t1 cascade; +NOTICE: drop cascades to table t1c drop table t2; drop table t3; -- diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 39ef5287557..7742a5af234 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -362,7 +362,15 @@ group by t1.a,t1.b,t1.c,t1.d,t2.x,t2.z; -- Cannot optimize when PK is deferrable explain (costs off) select * from t3 group by a,b,c; -drop table t1; +create temp table t1c () inherits (t1); + +-- Ensure we don't remove any columns when t1 has a child table +explain (costs off) select * from t1 group by a,b,c,d; + +-- Okay to remove columns if we're only querying the parent. +explain (costs off) select * from only t1 group by a,b,c,d; + +drop table t1 cascade; drop table t2; drop table t3;