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 18ce3a4ab'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 files changed, 27 insertions, 47 deletions

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 9ff80b8b403..af99e65aa7d 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -62,9 +62,6 @@ static Node *transformJoinOnClause(ParseState *pstate, JoinExpr *j,
 static RangeTblEntry *getRTEForSpecialRelationTypes(ParseState *pstate,
 							  RangeVar *rv);
 static RangeTblEntry *transformTableEntry(ParseState *pstate, RangeVar *r);
-static RangeTblEntry *transformCTEReference(ParseState *pstate, RangeVar *r,
-					  CommonTableExpr *cte, Index levelsup);
-static RangeTblEntry *transformENRReference(ParseState *pstate, RangeVar *r);
 static RangeTblEntry *transformRangeSubselect(ParseState *pstate,
 						RangeSubselect *r);
 static RangeTblEntry *transformRangeFunction(ParseState *pstate,
@@ -184,9 +181,12 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
 	RangeTblEntry *rte;
 	int			rtindex;
 
-	/* So far special relations are immutable; so they cannot be targets. */
-	rte = getRTEForSpecialRelationTypes(pstate, relation);
-	if (rte != NULL)
+	/*
+	 * ENRs hide tables of the same name, so we need to check for them first.
+	 * In contrast, CTEs don't hide tables (for this purpose).
+	 */
+	if (relation->schemaname == NULL &&
+		scanNameSpaceForENR(pstate, relation->relname))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("relation \"%s\" cannot be the target of a modifying statement",
@@ -431,35 +431,6 @@ transformTableEntry(ParseState *pstate, RangeVar *r)
 }
 
 /*
- * transformCTEReference --- transform a RangeVar that references a common
- * table expression (ie, a sub-SELECT defined in a WITH clause)
- */
-static RangeTblEntry *
-transformCTEReference(ParseState *pstate, RangeVar *r,
-					  CommonTableExpr *cte, Index levelsup)
-{
-	RangeTblEntry *rte;
-
-	rte = addRangeTableEntryForCTE(pstate, cte, levelsup, r, true);
-
-	return rte;
-}
-
-/*
- * transformENRReference --- transform a RangeVar that references an ephemeral
- * named relation
- */
-static RangeTblEntry *
-transformENRReference(ParseState *pstate, RangeVar *r)
-{
-	RangeTblEntry *rte;
-
-	rte = addRangeTableEntryForENR(pstate, r, true);
-
-	return rte;
-}
-
-/*
  * transformRangeSubselect --- transform a sub-SELECT appearing in FROM
  */
 static RangeTblEntry *
@@ -1071,19 +1042,32 @@ transformRangeTableSample(ParseState *pstate, RangeTableSample *rts)
 	return tablesample;
 }
 
-
+/*
+ * getRTEForSpecialRelationTypes
+ *
+ * If given RangeVar refers to a CTE or an EphemeralNamedRelation,
+ * build and return an appropriate RTE, otherwise return NULL
+ */
 static RangeTblEntry *
 getRTEForSpecialRelationTypes(ParseState *pstate, RangeVar *rv)
 {
 	CommonTableExpr *cte;
 	Index		levelsup;
-	RangeTblEntry *rte = NULL;
+	RangeTblEntry *rte;
+
+	/*
+	 * if it is a qualified name, it can't be a CTE or tuplestore reference
+	 */
+	if (rv->schemaname)
+		return NULL;
 
 	cte = scanNameSpaceForCTE(pstate, rv->relname, &levelsup);
 	if (cte)
-		rte = transformCTEReference(pstate, rv, cte, levelsup);
-	if (!rte && scanNameSpaceForENR(pstate, rv->relname))
-		rte = transformENRReference(pstate, rv);
+		rte = addRangeTableEntryForCTE(pstate, cte, levelsup, rv, true);
+	else if (scanNameSpaceForENR(pstate, rv->relname))
+		rte = addRangeTableEntryForENR(pstate, rv, true);
+	else
+		rte = NULL;
 
 	return rte;
 }
@@ -1119,15 +1103,11 @@ transformFromClauseItem(ParseState *pstate, Node *n,
 		/* Plain relation reference, or perhaps a CTE reference */
 		RangeVar   *rv = (RangeVar *) n;
 		RangeTblRef *rtr;
-		RangeTblEntry *rte = NULL;
+		RangeTblEntry *rte;
 		int			rtindex;
 
-		/*
-		 * if it is an unqualified name, it might be a CTE or tuplestore
-		 * reference
-		 */
-		if (!rv->schemaname)
-			rte = getRTEForSpecialRelationTypes(pstate, rv);
+		/* Check if it's a CTE or tuplestore reference */
+		rte = getRTEForSpecialRelationTypes(pstate, rv);
 
 		/* if not found above, must be a table reference */
 		if (!rte)