From b0fb2c6aa5a485e28210e13ae5536c1231b1261f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 27 Sep 2025 17:17:51 -0400 Subject: Refactor to avoid code duplication in transformPLAssignStmt. transformPLAssignStmt contained many lines cribbed directly from transformSelectStmt. I had supposed that we could manage to keep the two copies in sync, but the bug just fixed in 7504d2be9 shows that that hope was foolish. Let's refactor so there's just one copy. The main stumbling block to doing this is that transformPLAssignStmt has a chunk of custom code that has to run after transformTargetList but before we potentially modify the tlist further during analysis of ORDER BY and GROUP BY. Rather than make transformSelectStmt fully aware of PLAssignStmt processing, I put that code into a callback function. It still feels a little bit ugly, but it's not too awful, and surely it's better than a hundred lines of duplicated code. The steps involved in processing a PLAssignStmt remain exactly the same as before, just in different places. Author: Tom Lane Discussion: https://postgr.es/m/31027.1758919078@sss.pgh.pa.us --- src/backend/parser/analyze.c | 207 +++++++++++++++++-------------------------- 1 file changed, 83 insertions(+), 124 deletions(-) (limited to 'src/backend/parser/analyze.c') diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 70c6295ccd1..5aeb54eb5f6 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -55,6 +55,14 @@ #include "utils/syscache.h" +/* Passthrough data for transformPLAssignStmtTarget */ +typedef struct SelectStmtPassthrough +{ + PLAssignStmt *stmt; /* the assignment statement */ + Node *target; /* node representing the target variable */ + List *indirection; /* indirection yet to be applied to target */ +} SelectStmtPassthrough; + /* Hook for plugins to get control at end of parse analysis */ post_parse_analyze_hook_type post_parse_analyze_hook = NULL; @@ -64,7 +72,8 @@ static Query *transformInsertStmt(ParseState *pstate, InsertStmt *stmt); static OnConflictExpr *transformOnConflictClause(ParseState *pstate, OnConflictClause *onConflictClause); static int count_rowexpr_columns(ParseState *pstate, Node *expr); -static Query *transformSelectStmt(ParseState *pstate, SelectStmt *stmt); +static Query *transformSelectStmt(ParseState *pstate, SelectStmt *stmt, + SelectStmtPassthrough *passthru); static Query *transformValuesClause(ParseState *pstate, SelectStmt *stmt); static Query *transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt); static Node *transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, @@ -75,6 +84,8 @@ static Query *transformReturnStmt(ParseState *pstate, ReturnStmt *stmt); static Query *transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt); static Query *transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt); +static List *transformPLAssignStmtTarget(ParseState *pstate, List *tlist, + SelectStmtPassthrough *passthru); static Query *transformDeclareCursorStmt(ParseState *pstate, DeclareCursorStmt *stmt); static Query *transformExplainStmt(ParseState *pstate, @@ -371,7 +382,7 @@ transformStmt(ParseState *pstate, Node *parseTree) if (n->valuesLists) result = transformValuesClause(pstate, n); else if (n->op == SETOP_NONE) - result = transformSelectStmt(pstate, n); + result = transformSelectStmt(pstate, n, NULL); else result = transformSetOperationStmt(pstate, n); } @@ -1374,11 +1385,19 @@ count_rowexpr_columns(ParseState *pstate, Node *expr) * transformSelectStmt - * transforms a Select Statement * + * This function is also used to transform the source expression of a + * PLAssignStmt. In that usage, passthru is non-NULL and we need to + * call transformPLAssignStmtTarget after the initial transformation of the + * SELECT's targetlist. (We could generalize this into an arbitrary callback + * function, but for now that would just be more notation with no benefit.) + * All the rest is the same as a regular SelectStmt. + * * Note: this covers only cases with no set operations and no VALUES lists; * see below for the other cases. */ static Query * -transformSelectStmt(ParseState *pstate, SelectStmt *stmt) +transformSelectStmt(ParseState *pstate, SelectStmt *stmt, + SelectStmtPassthrough *passthru) { Query *qry = makeNode(Query); Node *qual; @@ -1415,8 +1434,16 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt) qry->targetList = transformTargetList(pstate, stmt->targetList, EXPR_KIND_SELECT_TARGET); - /* mark column origins */ - markTargetListOrigins(pstate, qry->targetList); + /* + * If we're within a PLAssignStmt, do further transformation of the + * targetlist; that has to happen before we consider sorting or grouping. + * Otherwise, mark column origins (which are useless in a PLAssignStmt). + */ + if (passthru) + qry->targetList = transformPLAssignStmtTarget(pstate, qry->targetList, + passthru); + else + markTargetListOrigins(pstate, qry->targetList); /* transform WHERE */ qual = transformWhereClause(pstate, stmt->whereClause, @@ -2763,20 +2790,13 @@ transformReturningClause(ParseState *pstate, Query *qry, static Query * transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) { - Query *qry = makeNode(Query); + Query *qry; ColumnRef *cref = makeNode(ColumnRef); List *indirection = stmt->indirection; int nnames = stmt->nnames; - SelectStmt *sstmt = stmt->val; Node *target; - Oid targettype; - int32 targettypmod; - Oid targetcollation; - List *tlist; - TargetEntry *tle; - Oid type_id; - Node *qual; - ListCell *l; + SelectStmtPassthrough passthru; + bool save_resolve_unknowns; /* * First, construct a ColumnRef for the target variable. If the target @@ -2802,33 +2822,62 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) /* * Transform the target reference. Typically we will get back a Param - * node, but there's no reason to be too picky about its type. + * node, but there's no reason to be too picky about its type. (Note that + * we must do this before calling transformSelectStmt. It's tempting to + * do it inside transformPLAssignStmtTarget, but we need to do it before + * adding any FROM tables to the pstate's namespace, else we might wrongly + * resolve the target as a table column.) */ target = transformExpr(pstate, (Node *) cref, EXPR_KIND_UPDATE_TARGET); - targettype = exprType(target); - targettypmod = exprTypmod(target); - targetcollation = exprCollation(target); + + /* Set up passthrough data for transformPLAssignStmtTarget */ + passthru.stmt = stmt; + passthru.target = target; + passthru.indirection = indirection; /* - * The rest mostly matches transformSelectStmt, except that we needn't - * consider WITH or INTO, and we build a targetlist our own way. + * To avoid duplicating a lot of code, we use transformSelectStmt to do + * almost all of the work. However, we need to do additional processing + * on the SELECT's targetlist after it's been transformed, but before + * possible addition of targetlist items for ORDER BY or GROUP BY. + * transformSelectStmt knows it should call transformPLAssignStmtTarget if + * it's passed a passthru argument. + * + * Also, disable resolution of unknown-type tlist items; PL/pgSQL wants to + * deal with that itself. */ - qry->commandType = CMD_SELECT; - pstate->p_is_insert = false; + save_resolve_unknowns = pstate->p_resolve_unknowns; + pstate->p_resolve_unknowns = false; + qry = transformSelectStmt(pstate, stmt->val, &passthru); + pstate->p_resolve_unknowns = save_resolve_unknowns; - /* make FOR UPDATE/FOR SHARE info available to addRangeTableEntry */ - pstate->p_locking_clause = sstmt->lockingClause; - - /* make WINDOW info available for window functions, too */ - pstate->p_windowdefs = sstmt->windowClause; + return qry; +} - /* process the FROM clause */ - transformFromClause(pstate, sstmt->fromClause); +/* + * Callback function to adjust a SELECT's tlist to make the output suitable + * for assignment to a PLAssignStmt's target variable. + * + * Note: we actually modify the tle->expr in-place, but the function's API + * is set up to not presume that. + */ +static List * +transformPLAssignStmtTarget(ParseState *pstate, List *tlist, + SelectStmtPassthrough *passthru) +{ + PLAssignStmt *stmt = passthru->stmt; + Node *target = passthru->target; + List *indirection = passthru->indirection; + Oid targettype; + int32 targettypmod; + Oid targetcollation; + TargetEntry *tle; + Oid type_id; - /* initially transform the targetlist as if in SELECT */ - tlist = transformTargetList(pstate, sstmt->targetList, - EXPR_KIND_SELECT_TARGET); + targettype = exprType(target); + targettypmod = exprTypmod(target); + targetcollation = exprCollation(target); /* we should have exactly one targetlist item */ if (list_length(tlist) != 1) @@ -2906,97 +2955,7 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt) pstate->p_expr_kind = EXPR_KIND_NONE; - qry->targetList = list_make1(tle); - - /* transform WHERE */ - qual = transformWhereClause(pstate, sstmt->whereClause, - EXPR_KIND_WHERE, "WHERE"); - - /* initial processing of HAVING clause is much like WHERE clause */ - qry->havingQual = transformWhereClause(pstate, sstmt->havingClause, - EXPR_KIND_HAVING, "HAVING"); - - /* - * Transform sorting/grouping stuff. Do ORDER BY first because both - * transformGroupClause and transformDistinctClause need the results. Note - * that these functions can also change the targetList, so it's passed to - * them by reference. - */ - qry->sortClause = transformSortClause(pstate, - sstmt->sortClause, - &qry->targetList, - EXPR_KIND_ORDER_BY, - false /* allow SQL92 rules */ ); - - qry->groupClause = transformGroupClause(pstate, - sstmt->groupClause, - &qry->groupingSets, - &qry->targetList, - qry->sortClause, - EXPR_KIND_GROUP_BY, - false /* allow SQL92 rules */ ); - qry->groupDistinct = sstmt->groupDistinct; - - if (sstmt->distinctClause == NIL) - { - qry->distinctClause = NIL; - qry->hasDistinctOn = false; - } - else if (linitial(sstmt->distinctClause) == NULL) - { - /* We had SELECT DISTINCT */ - qry->distinctClause = transformDistinctClause(pstate, - &qry->targetList, - qry->sortClause, - false); - qry->hasDistinctOn = false; - } - else - { - /* We had SELECT DISTINCT ON */ - qry->distinctClause = transformDistinctOnClause(pstate, - sstmt->distinctClause, - &qry->targetList, - qry->sortClause); - qry->hasDistinctOn = true; - } - - /* transform LIMIT */ - qry->limitOffset = transformLimitClause(pstate, sstmt->limitOffset, - EXPR_KIND_OFFSET, "OFFSET", - sstmt->limitOption); - qry->limitCount = transformLimitClause(pstate, sstmt->limitCount, - EXPR_KIND_LIMIT, "LIMIT", - sstmt->limitOption); - qry->limitOption = sstmt->limitOption; - - /* transform window clauses after we have seen all window functions */ - qry->windowClause = transformWindowDefinitions(pstate, - pstate->p_windowdefs, - &qry->targetList); - - qry->rtable = pstate->p_rtable; - qry->rteperminfos = pstate->p_rteperminfos; - qry->jointree = makeFromExpr(pstate->p_joinlist, qual); - - qry->hasSubLinks = pstate->p_hasSubLinks; - qry->hasWindowFuncs = pstate->p_hasWindowFuncs; - qry->hasTargetSRFs = pstate->p_hasTargetSRFs; - qry->hasAggs = pstate->p_hasAggs; - - foreach(l, sstmt->lockingClause) - { - transformLockingClause(pstate, qry, - (LockingClause *) lfirst(l), false); - } - - assign_query_collations(pstate, qry); - - /* this must be done after collations, for reliable comparison of exprs */ - if (pstate->p_hasAggs || qry->groupClause || qry->groupingSets || qry->havingQual) - parseCheckAggregates(pstate, qry); - - return qry; + return list_make1(tle); } -- cgit v1.2.3