Skip to content

Commit 7421f4b

Browse files
tglsfdctglsfdc
committed
Fix incorrect handling of CTEs and ENRs as DML target relations.
setTargetTable threw an error if the proposed target RangeVar's relname matched any visible CTE or ENR. This breaks backwards compatibility in the CTE case, since pre-v10 we never looked for a CTE here at all, so that CTE names did not mask regular tables. It does seem like a good idea to throw an error for the ENR case, though, thus causing ENRs to mask tables for this purpose; ENRs are new in v10 so we're not breaking existing code, and we may someday want to allow them to be the targets of DML. To fix that, replace use of getRTEForSpecialRelationTypes, which was overkill anyway, with use of scanNameSpaceForENR. A second problem was that the check neglected to verify null schemaname, so that a CTE or ENR could incorrectly be thought to match a qualified RangeVar. That happened because getRTEForSpecialRelationTypes relied on its caller to have checked for null schemaname. Even though the one remaining caller got it right, this is obviously bug-prone, so move the check inside getRTEForSpecialRelationTypes. Also, revert commit 18ce3a4's extremely poorly thought out decision to add a NULL return case to parserOpenTable --- without either documenting that or adjusting any of the callers to check for it. The current bug seems to have arisen in part due to working around that bad idea. In passing, remove the one-line shim functions transformCTEReference and transformENRReference --- they don't seem to be adding any clarity or functionality. Per report from Hugo Mercier (via Julien Rouhaud). Back-patch to v10 where the bug was introduced. Thomas Munro, with minor editing by me Discussion: https://postgr.es/m/CAOBaU_YdPVH+PTtiKSSLOiiW3mVDYsnNUekK+XPbHXiP=wrFLA@mail.gmail.com
1 parent 4211673 commit 7421f4b

File tree

6 files changed

+59
-63
lines changed

6 files changed

+59
-63
lines changed

‎src/backend/parser/parse_clause.c

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ static Node *transformJoinOnClause(ParseState *pstate, JoinExpr *j,
6262
static RangeTblEntry *getRTEForSpecialRelationTypes(ParseState *pstate,
6363
RangeVar *rv);
6464
static RangeTblEntry *transformTableEntry(ParseState *pstate, RangeVar *r);
65-
static RangeTblEntry *transformCTEReference(ParseState *pstate, RangeVar *r,
66-
CommonTableExpr *cte, Index levelsup);
67-
static RangeTblEntry *transformENRReference(ParseState *pstate, RangeVar *r);
6865
static RangeTblEntry *transformRangeSubselect(ParseState *pstate,
6966
RangeSubselect *r);
7067
static RangeTblEntry *transformRangeFunction(ParseState *pstate,
@@ -184,9 +181,12 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
184181
RangeTblEntry *rte;
185182
int rtindex;
186183

187-
/* So far special relations are immutable; so they cannot be targets. */
188-
rte = getRTEForSpecialRelationTypes(pstate, relation);
189-
if (rte != NULL)
184+
/*
185+
* ENRs hide tables of the same name, so we need to check for them first.
186+
* In contrast, CTEs don't hide tables (for this purpose).
187+
*/
188+
if (relation->schemaname == NULL &&
189+
scanNameSpaceForENR(pstate, relation->relname))
190190
ereport(ERROR,
191191
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
192192
errmsg("relation \"%s\" cannot be the target of a modifying statement",
@@ -430,35 +430,6 @@ transformTableEntry(ParseState *pstate, RangeVar *r)
430430
return rte;
431431
}
432432

433-
/*
434-
* transformCTEReference --- transform a RangeVar that references a common
435-
* table expression (ie, a sub-SELECT defined in a WITH clause)
436-
*/
437-
static RangeTblEntry *
438-
transformCTEReference(ParseState *pstate, RangeVar *r,
439-
CommonTableExpr *cte, Index levelsup)
440-
{
441-
RangeTblEntry *rte;
442-
443-
rte = addRangeTableEntryForCTE(pstate, cte, levelsup, r, true);
444-
445-
return rte;
446-
}
447-
448-
/*
449-
* transformENRReference --- transform a RangeVar that references an ephemeral
450-
* named relation
451-
*/
452-
static RangeTblEntry *
453-
transformENRReference(ParseState *pstate, RangeVar *r)
454-
{
455-
RangeTblEntry *rte;
456-
457-
rte = addRangeTableEntryForENR(pstate, r, true);
458-
459-
return rte;
460-
}
461-
462433
/*
463434
* transformRangeSubselect --- transform a sub-SELECT appearing in FROM
464435
*/
@@ -1071,19 +1042,32 @@ transformRangeTableSample(ParseState *pstate, RangeTableSample *rts)
10711042
return tablesample;
10721043
}
10731044

1074-
1045+
/*
1046+
* getRTEForSpecialRelationTypes
1047+
*
1048+
* If given RangeVar refers to a CTE or an EphemeralNamedRelation,
1049+
* build and return an appropriate RTE, otherwise return NULL
1050+
*/
10751051
static RangeTblEntry *
10761052
getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
10771053
{
10781054
CommonTableExpr *cte;
10791055
Index levelsup;
1080-
RangeTblEntry *rte = NULL;
1056+
RangeTblEntry *rte;
1057+
1058+
/*
1059+
* if it is a qualified name, it can't be a CTE or tuplestore reference
1060+
*/
1061+
if (rv->schemaname)
1062+
return NULL;
10811063

10821064
cte = scanNameSpaceForCTE(pstate, rv->relname, &levelsup);
10831065
if (cte)
1084-
rte = transformCTEReference(pstate, rv, cte, levelsup);
1085-
if (!rte && scanNameSpaceForENR(pstate, rv->relname))
1086-
rte = transformENRReference(pstate, rv);
1066+
rte = addRangeTableEntryForCTE(pstate, cte, levelsup, rv, true);
1067+
else if (scanNameSpaceForENR(pstate, rv->relname))
1068+
rte = addRangeTableEntryForENR(pstate, rv, true);
1069+
else
1070+
rte = NULL;
10871071

10881072
return rte;
10891073
}
@@ -1119,15 +1103,11 @@ transformFromClauseItem(ParseState *pstate, Node *n,
11191103
/* Plain relation reference, or perhaps a CTE reference */
11201104
RangeVar *rv = (RangeVar *) n;
11211105
RangeTblRef *rtr;
1122-
RangeTblEntry *rte = NULL;
1106+
RangeTblEntry *rte;
11231107
int rtindex;
11241108

1125-
/*
1126-
* if it is an unqualified name, it might be a CTE or tuplestore
1127-
* reference
1128-
*/
1129-
if (!rv->schemaname)
1130-
rte = getRTEForSpecialRelationTypes(pstate, rv);
1109+
/* Check if it's a CTE or tuplestore reference */
1110+
rte = getRTEForSpecialRelationTypes(pstate, rv);
11311111

11321112
/* if not found above, must be a table reference */
11331113
if (!rte)

‎src/backend/parser/parse_relation.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,19 +1159,13 @@ parserOpenTable(ParseState *pstate, const RangeVar *relation, int lockmode)
11591159
relation->schemaname, relation->relname)));
11601160
else
11611161
{
1162-
/*
1163-
* An unqualified name might be a named ephemeral relation.
1164-
*/
1165-
if (get_visible_ENR_metadata(pstate->p_queryEnv, relation->relname))
1166-
rel = NULL;
1167-
11681162
/*
11691163
* An unqualified name might have been meant as a reference to
11701164
* some not-yet-in-scope CTE. The bare "does not exist" message
11711165
* has proven remarkably unhelpful for figuring out such problems,
11721166
* so we take pains to offer a specific hint.
11731167
*/
1174-
else if (isFutureCTE(pstate, relation->relname))
1168+
if (isFutureCTE(pstate, relation->relname))
11751169
ereport(ERROR,
11761170
(errcode(ERRCODE_UNDEFINED_TABLE),
11771171
errmsg("relation \"%s\" does not exist",

‎src/test/regress/expected/plpgsql.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5879,19 +5879,19 @@ CREATE FUNCTION transition_table_level2_bad_usage_func()
58795879
LANGUAGE plpgsql
58805880
AS $$
58815881
BEGIN
5882-
INSERT INTO d VALUES (1000000, 1000000, 'x');
5882+
INSERT INTO dx VALUES (1000000, 1000000, 'x');
58835883
RETURN NULL;
58845884
END;
58855885
$$;
58865886
CREATE TRIGGER transition_table_level2_bad_usage_trigger
58875887
AFTER DELETE ON transition_table_level2
5888-
REFERENCING OLD TABLE AS d
5888+
REFERENCING OLD TABLE AS dx
58895889
FOR EACH STATEMENT EXECUTE PROCEDURE
58905890
transition_table_level2_bad_usage_func();
58915891
DELETE FROM transition_table_level2
58925892
WHERE level2_no BETWEEN 301 AND 305;
5893-
ERROR: relation "d" cannot be the target of a modifying statement
5894-
CONTEXT: SQL statement "INSERT INTO d VALUES (1000000, 1000000, 'x')"
5893+
ERROR: relation "dx" cannot be the target of a modifying statement
5894+
CONTEXT: SQL statement "INSERT INTO dx VALUES (1000000, 1000000, 'x')"
58955895
PL/pgSQL function transition_table_level2_bad_usage_func() line 3 at SQL statement
58965896
DROP TRIGGER transition_table_level2_bad_usage_trigger
58975897
ON transition_table_level2;

‎src/test/regress/expected/with.out

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2273,5 +2273,19 @@ with ordinality as (select 1 as x) select * from ordinality;
22732273
(1 row)
22742274

22752275
-- check sane response to attempt to modify CTE relation
2276-
WITH d AS (SELECT 42) INSERT INTO d VALUES (1);
2277-
ERROR: relation "d" cannot be the target of a modifying statement
2276+
WITH test AS (SELECT 42) INSERT INTO test VALUES (1);
2277+
ERROR: relation "test" does not exist
2278+
LINE 1: WITH test AS (SELECT 42) INSERT INTO test VALUES (1);
2279+
^
2280+
-- check response to attempt to modify table with same name as a CTE (perhaps
2281+
-- surprisingly it works, because CTEs don't hide tables from data-modifying
2282+
-- statements)
2283+
create table test (i int);
2284+
with test as (select 42) insert into test select * from test;
2285+
select * from test;
2286+
i
2287+
----
2288+
42
2289+
(1 row)
2290+
2291+
drop table test;

‎src/test/regress/sql/plpgsql.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4678,14 +4678,14 @@ CREATE FUNCTION transition_table_level2_bad_usage_func()
46784678
LANGUAGE plpgsql
46794679
AS $$
46804680
BEGIN
4681-
INSERT INTO d VALUES (1000000, 1000000, 'x');
4681+
INSERT INTO dx VALUES (1000000, 1000000, 'x');
46824682
RETURN NULL;
46834683
END;
46844684
$$;
46854685

46864686
CREATE TRIGGER transition_table_level2_bad_usage_trigger
46874687
AFTER DELETE ON transition_table_level2
4688-
REFERENCING OLD TABLE AS d
4688+
REFERENCING OLD TABLE AS dx
46894689
FOR EACH STATEMENT EXECUTE PROCEDURE
46904690
transition_table_level2_bad_usage_func();
46914691

‎src/test/regress/sql/with.sql

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1030,4 +1030,12 @@ create table foo (with ordinality); -- fail, WITH is a reserved word
10301030
with ordinality as (select 1 as x) select * from ordinality;
10311031

10321032
-- check sane response to attempt to modify CTE relation
1033-
WITH d AS (SELECT 42) INSERT INTO d VALUES (1);
1033+
WITH test AS (SELECT 42) INSERT INTO test VALUES (1);
1034+
1035+
-- check response to attempt to modify table with same name as a CTE (perhaps
1036+
-- surprisingly it works, because CTEs don't hide tables from data-modifying
1037+
-- statements)
1038+
create table test (i int);
1039+
with test as (select 42) insert into test select * from test;
1040+
select * from test;
1041+
drop table test;

0 commit comments

Comments
 (0)