From bf7ca15875988a88e97302e012d7c4808bef3ea9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 10 Nov 2014 15:21:09 -0500 Subject: Ensure that RowExprs and whole-row Vars produce the expected column names. At one time it wasn't terribly important what column names were associated with the fields of a composite Datum, but since the introduction of operations like row_to_json(), it's important that looking up the rowtype ID embedded in the Datum returns the column names that users would expect. That did not work terribly well before this patch: you could get the column names of the underlying table, or column aliases from any level of the query, depending on minor details of the plan tree. You could even get totally empty field names, which is disastrous for cases like row_to_json(). To fix this for whole-row Vars, look to the RTE referenced by the Var, and make sure its column aliases are applied to the rowtype associated with the result Datums. This is a tad scary because we might have to return a transient RECORD type even though the Var is declared as having some named rowtype. In principle it should be all right because the record type will still be physically compatible with the named rowtype; but I had to weaken one Assert in ExecEvalConvertRowtype, and there might be third-party code containing similar assumptions. Similarly, RowExprs have to be willing to override the column names coming from a named composite result type and produce a RECORD when the column aliases visible at the site of the RowExpr differ from the underlying table's column names. In passing, revert the decision made in commit 398f70ec070fe601 to add an alias-list argument to ExecTypeFromExprList: better to provide that functionality in a separate function. This also reverts most of the code changes in d68581483564ec0f, which we don't need because we're no longer depending on the tupdesc found in the child plan node's result slot to be blessed. Back-patch to 9.4, but not earlier, since this solution changes the results in some cases that users might not have realized were buggy. We'll apply a more restricted form of this patch in older branches. --- src/backend/executor/nodeFunctionscan.c | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'src/backend/executor/nodeFunctionscan.c') diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c index 945a414e96f..46417081086 100644 --- a/src/backend/executor/nodeFunctionscan.c +++ b/src/backend/executor/nodeFunctionscan.c @@ -26,7 +26,6 @@ #include "executor/nodeFunctionscan.h" #include "funcapi.h" #include "nodes/nodeFuncs.h" -#include "parser/parsetree.h" #include "utils/builtins.h" #include "utils/memutils.h" @@ -279,8 +278,6 @@ FunctionScanState * ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags) { FunctionScanState *scanstate; - RangeTblEntry *rte = rt_fetch(node->scan.scanrelid, - estate->es_range_table); int nfuncs = list_length(node->functions); TupleDesc scan_tupdesc; int i, @@ -494,22 +491,6 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags) Assert(attno == natts); } - /* - * Make sure the scan result tupdesc has the column names the query - * expects. This affects the output of constructs like row_to_json which - * read the column names from the passed-in tupdesc. - */ - i = 0; - foreach(lc, rte->eref->colnames) - { - char *attname = strVal(lfirst(lc)); - - if (i >= scan_tupdesc->natts) - break; /* shouldn't happen, but just in case */ - namestrcpy(&(scan_tupdesc->attrs[i]->attname), attname); - i++; - } - ExecAssignScanType(&scanstate->ss, scan_tupdesc); /* -- cgit v1.2.3