git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: eb89cb4)

Reject SELECT ... GROUP BY GROUPING SETS (()) FOR UPDATE.

author Tom Lane <tgl@sss.pgh.pa.us>

Tue, 1 Jun 2021 15:12:56 +0000 (11:12 -0400)

committer Tom Lane <tgl@sss.pgh.pa.us>

Tue, 1 Jun 2021 15:12:56 +0000 (11:12 -0400)

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 patch | blob | blame | history

src/backend/utils/adt/selfuncs.c patch | blob | blame | history

src/test/regress/expected/errors.out patch | blob | blame | history

src/test/regress/sql/errors.sql patch | blob | blame | history

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c

index 201b88d1adb1d459844625c06c6182fc27eabf54..9cede29d6a816576cea0f69de15a8ee9f92fc187 100644 (file)

--- a/src/backend/parser/analyze.c

+++ b/src/backend/parser/analyze.c

@@ -3019,7 +3019,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 37ddda7724012687d64f846bce92520655835f16..0c8c05f6c2e8e8c21d40f1d1dafdca40d8123a9f 100644 (file)

--- a/src/backend/utils/adt/selfuncs.c

+++ b/src/backend/utils/adt/selfuncs.c

@@ -5497,7 +5497,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 1e7b5a70461978bc230d2a8043920ea39d64aa5d..15862d44753e07b0d59f13a56f8e4b02ab041750 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 66a56b28f62b8811dab73b5d3cb8b605b52cabb6..a7fcf72dd4509e585edef887c5ed724ee162530d 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

This is the main PostgreSQL git repository.