From e5b0fffa17f610f03f03952c8c4a247c39e61292 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 1 Jun 2021 11:12:56 -0400 Subject: [PATCH] Reject SELECT ... GROUP BY GROUPING SETS (()) FOR UPDATE. This case should be disallowed, just as FOR UPDATE with a plain GROUP BY is disallowed; FOR UPDATE only makes sense when each row of the query result can be identified with a single table row. However, we missed teaching CheckSelectLocking() to check groupingSets as well as groupClause, so that it would allow degenerate grouping sets. That resulted in a bad plan and a null-pointer dereference in the executor. Looking around for other instances of the same bug, the only one I found was in examine_simple_variable(). That'd just lead to silly estimates, but it should be fixed too. Per private report from Yaoguang Chen. Back-patch to all supported branches. --- src/backend/parser/analyze.c | 2 +- src/backend/utils/adt/selfuncs.c | 3 ++- src/test/regress/expected/errors.out | 5 +++++ src/test/regress/sql/errors.sql | 4 ++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 523baf08cd..d09e8ccde4 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -2688,7 +2688,7 @@ CheckSelectLocking(Query *qry, LockClauseStrength strength) translator: %s is a SQL row locking clause such as FOR UPDATE */ errmsg("%s is not allowed with DISTINCT clause", LCS_asString(strength)))); - if (qry->groupClause != NIL) + if (qry->groupClause != NIL || qry->groupingSets != NIL) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), /*------ diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index e96bfce4dc..c2aeb4b947 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -5150,7 +5150,8 @@ examine_simple_variable(PlannerInfo *root, Var *var, * of learning something even with it. */ if (subquery->setOperations || - subquery->groupClause) + subquery->groupClause || + subquery->groupingSets) return; /* diff --git a/src/test/regress/expected/errors.out b/src/test/regress/expected/errors.out index 1e7b5a7046..15862d4475 100644 --- a/src/test/regress/expected/errors.out +++ b/src/test/regress/expected/errors.out @@ -50,6 +50,11 @@ select distinct on (foobar) * from pg_database; ERROR: column "foobar" does not exist LINE 1: select distinct on (foobar) * from pg_database; ^ +-- grouping with FOR UPDATE +select null from pg_database group by datname for update; +ERROR: FOR UPDATE is not allowed with GROUP BY clause +select null from pg_database group by grouping sets (()) for update; +ERROR: FOR UPDATE is not allowed with GROUP BY clause -- -- DELETE -- missing relation name (this had better not wildcard!) diff --git a/src/test/regress/sql/errors.sql b/src/test/regress/sql/errors.sql index 66a56b28f6..a7fcf72dd4 100644 --- a/src/test/regress/sql/errors.sql +++ b/src/test/regress/sql/errors.sql @@ -37,6 +37,10 @@ select * from pg_database where pg_database.datname = nonesuch; -- bad attribute name in select distinct on select distinct on (foobar) * from pg_database; +-- grouping with FOR UPDATE +select null from pg_database group by datname for update; +select null from pg_database group by grouping sets (()) for update; + -- -- DELETE