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.

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 5eb3ec3e205758a875244cb586b6500eb08c4d51..f491d9373976085755811489c1f3d7b5f7ca50e1 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2693,7 +2693,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 2240636e6c42ec936bb662c59c5df889f3753bc3..33d6ee714a8c3113416101fe434428a62333cd53 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5227,7 +5227,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 ce473a03efd9d8b80c4f116d8bd7d315e65a5978..cc1cdc0dd02240f9dfc7f27e60c9e68c2fd9b7fe 100644 (file)
--- 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 14bc723a52a80887fb8adcf0457babc5a69be8e5..9444a5d48f3aea7a15f61571e5488b0733826a68 100644 (file)
--- 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