Feedback-ID: ia2694551:Fastmail
Date: Sat, 5 Apr 2025 15:03:08 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: luy70@psu.edu, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18877: PostgreSQL triggers assertion failure
Message-ID: <202504051303.fhpbodkciqry@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ibz3cgmi3xh26lo3"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2387814.1743825262@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/202504051303.fhpbodkciqry%40alvherre.pgsql>
Precedence: bulk


--ibz3cgmi3xh26lo3
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Apr-04, Tom Lane wrote:

> However ... it's possible that this isn't the stupidest, most
> brute-force code in Postgres, but I'll wager it's in the top ten.
> Is it really necessary to transform the subquery twice just to
> produce this error?  It seems like the constructed EXPR_SUBLINK
> should produce the same error all by itself.  I tried nuking this
> whole stanza to see whether that would happen, and then there are a
> couple of regression test cases that don't produce the same results.
> But I feel like it could be made to work with a bit more thought.

Ugh, yeah, this is really dumb.  This quick-and-dirty patch (not final
form) gets us halfway there, by checking the targetlist after
transforming the query.  I think this is the behavior we want, although
it's a bit scary that we have so few test cases for this.

Anyway, if we only do this, this query from the regression tests still
doesn't work the way we want:

  SELECT JSON_ARRAY(SELECT FROM (VALUES (1)) foo(i));

It causes an error to be raised _during_ transformation, namely

+ERROR:  table "q" has 0 columns available but 1 columns specified

which we'd like to report differently.  I think we want to verify the
targetlist length of ctor->query before transformation, but I'm not sure
exactly how, yet.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

--ibz3cgmi3xh26lo3
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename="transformonce.patch"

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 9caf1e481a2..1dcbd70a509 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -3766,30 +3766,12 @@ transformJsonArrayQueryConstructor(ParseState *pstate,
 	ResTarget  *target = makeNode(ResTarget);
 	JsonArrayAgg *agg = makeNode(JsonArrayAgg);
 	ColumnRef  *colref = makeNode(ColumnRef);
-	Query	   *query;
-	ParseState *qpstate;
-
-	/* Transform query only for counting target list entries. */
-	qpstate = make_parsestate(pstate);
-
-	query = transformStmt(qpstate, ctor->query);
-
-	if (count_nonjunk_tlist_entries(query->targetList) != 1)
-		ereport(ERROR,
-				errcode(ERRCODE_SYNTAX_ERROR),
-				errmsg("subquery must return only one column"),
-				parser_errposition(pstate, ctor->location));
-
-	free_parsestate(qpstate);
+	Node	   *transformed;
 
 	colref->fields = list_make2(makeString(pstrdup("q")),
 								makeString(pstrdup("a")));
 	colref->location = ctor->location;
 
-	/*
-	 * No formatting necessary, so set formatted_expr to be the same as
-	 * raw_expr.
-	 */
 	agg->arg = makeJsonValueExpr((Expr *) colref, (Expr *) colref,
 								 ctor->format);
 	agg->absent_on_null = ctor->absent_on_null;
@@ -3820,7 +3802,32 @@ transformJsonArrayQueryConstructor(ParseState *pstate,
 	sublink->subselect = (Node *) select;
 	sublink->location = ctor->location;
 
-	return transformExprRecurse(pstate, (Node *) sublink);
+	transformed = transformExprRecurse(pstate, (Node *) sublink);
+
+	/*
+	 * Verify that the query (which has been installed as the subselect in the
+	 * sublink) returns exactly one column after transformation.
+	 */
+	{
+		Query	   *query;
+		RangeTblEntry *rte;
+
+		query = castNode(Query, (castNode(SubLink, transformed))->subselect);
+		Assert(list_length(query->rtable) == 1);
+
+		rte = linitial_node(RangeTblEntry, query->rtable);
+		Assert(rte->rtekind == RTE_SUBQUERY);
+
+		/* Transfer our attention to the subquery */
+		query = rte->subquery;
+		if (count_nonjunk_tlist_entries(query->targetList) != 1)
+			ereport(ERROR,
+					errcode(ERRCODE_SYNTAX_ERROR),
+					errmsg("subquery must return only one column"),
+					parser_errposition(pstate, ctor->location));
+	}
+
+	return transformed;
 }
 
 /*

--ibz3cgmi3xh26lo3--