diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index 7cfa63f37d..88af73575c 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -50,6 +50,7 @@ #include "nodes/nodeFuncs.h" #include "optimizer/planner.h" #include "parser/parse_coerce.h" +#include "parser/parsetree.h" #include "pgstat.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -712,6 +713,8 @@ ExecEvalWholeRowVar(WholeRowVarExprState *wrvstate, ExprContext *econtext, { Var *variable = (Var *) wrvstate->xprstate.expr; TupleTableSlot *slot; + TupleDesc output_tupdesc; + MemoryContext oldcontext; bool needslow = false; if (isDone) @@ -787,8 +790,6 @@ ExecEvalWholeRowVar(WholeRowVarExprState *wrvstate, ExprContext *econtext, /* If so, build the junkfilter in the query memory context */ if (junk_filter_needed) { - MemoryContext oldcontext; - oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); wrvstate->wrv_junkFilter = ExecInitJunkFilter(subplan->plan->targetlist, @@ -860,10 +861,60 @@ ExecEvalWholeRowVar(WholeRowVarExprState *wrvstate, ExprContext *econtext, needslow = true; /* need runtime check for null */ } + /* + * Use the variable's declared rowtype as the descriptor for the + * output values, modulo possibly assigning new column names below. In + * particular, we *must* absorb any attisdropped markings. + */ + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); + output_tupdesc = CreateTupleDescCopy(var_tupdesc); + MemoryContextSwitchTo(oldcontext); + ReleaseTupleDesc(var_tupdesc); } + else + { + /* + * In the RECORD case, we use the input slot's rowtype as the + * descriptor for the output values, modulo possibly assigning new + * column names below. + */ + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); + output_tupdesc = CreateTupleDescCopy(slot->tts_tupleDescriptor); + MemoryContextSwitchTo(oldcontext); + } - /* Skip the checking on future executions of node */ + /* + * Construct a tuple descriptor for the composite values we'll produce, + * and make sure its record type is "blessed". The main reason to do this + * is to be sure that operations such as row_to_json() will see the + * desired column names when they look up the descriptor from the type + * information embedded in the composite values. + * + * We already got the correct physical datatype info above, but now we + * should try to find the source RTE and adopt its column aliases, in case + * they are different from the original rowtype's names. For example, in + * "SELECT foo(t) FROM tab t(x,y)", the first two columns in the composite + * output should be named "x" and "y" regardless of tab's column names. + * + * If we can't locate the RTE, assume the column names we've got are OK. + * (As of this writing, the only cases where we can't locate the RTE are + * in execution of trigger WHEN clauses, and then the Var will have the + * trigger's relation's rowtype, so its names are fine.) + */ + if (econtext->ecxt_estate && + variable->varno <= list_length(econtext->ecxt_estate->es_range_table)) + { + RangeTblEntry *rte = rt_fetch(variable->varno, + econtext->ecxt_estate->es_range_table); + + ExecTypeSetColNames(output_tupdesc, rte->eref->colnames); + } + + /* Bless the tupdesc if needed, and save it in the execution state */ + wrvstate->wrv_tupdesc = BlessTupleDesc(output_tupdesc); + + /* Skip all the above on future executions of node */ if (needslow) wrvstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalWholeRowSlow; else @@ -886,7 +937,6 @@ ExecEvalWholeRowFast(WholeRowVarExprState *wrvstate, ExprContext *econtext, { Var *variable = (Var *) wrvstate->xprstate.expr; TupleTableSlot *slot; - TupleDesc slot_tupdesc; HeapTupleHeader dtuple; if (isDone) @@ -916,34 +966,16 @@ ExecEvalWholeRowFast(WholeRowVarExprState *wrvstate, ExprContext *econtext, if (wrvstate->wrv_junkFilter != NULL) slot = ExecFilterJunk(wrvstate->wrv_junkFilter, slot); - /* - * If it's a RECORD Var, we'll use the slot's type ID info. It's likely - * that the slot's type is also RECORD; if so, make sure it's been - * "blessed", so that the Datum can be interpreted later. (Note: we must - * do this here, not in ExecEvalWholeRowVar, because some plan trees may - * return different slots at different times. We have to be ready to - * bless additional slots during the run.) - */ - slot_tupdesc = slot->tts_tupleDescriptor; - if (variable->vartype == RECORDOID && - slot_tupdesc->tdtypeid == RECORDOID && - slot_tupdesc->tdtypmod < 0) - assign_record_type_typmod(slot_tupdesc); - /* * Copy the slot tuple and make sure any toasted fields get detoasted. */ dtuple = DatumGetHeapTupleHeader(ExecFetchSlotTupleDatum(slot)); /* - * If the Var identifies a named composite type, label the datum with that - * type; otherwise we'll use the slot's info. + * Label the datum with the composite type info we identified before. */ - if (variable->vartype != RECORDOID) - { - HeapTupleHeaderSetTypeId(dtuple, variable->vartype); - HeapTupleHeaderSetTypMod(dtuple, variable->vartypmod); - } + HeapTupleHeaderSetTypeId(dtuple, wrvstate->wrv_tupdesc->tdtypeid); + HeapTupleHeaderSetTypMod(dtuple, wrvstate->wrv_tupdesc->tdtypmod); return PointerGetDatum(dtuple); } @@ -997,8 +1029,9 @@ ExecEvalWholeRowSlow(WholeRowVarExprState *wrvstate, ExprContext *econtext, tuple = ExecFetchSlotTuple(slot); tupleDesc = slot->tts_tupleDescriptor; + /* wrv_tupdesc is a good enough representation of the Var's rowtype */ Assert(variable->vartype != RECORDOID); - var_tupdesc = lookup_rowtype_tupdesc(variable->vartype, -1); + var_tupdesc = wrvstate->wrv_tupdesc; /* Check to see if any dropped attributes are non-null */ for (i = 0; i < var_tupdesc->natts; i++) @@ -1025,12 +1058,10 @@ ExecEvalWholeRowSlow(WholeRowVarExprState *wrvstate, ExprContext *econtext, dtuple = DatumGetHeapTupleHeader(ExecFetchSlotTupleDatum(slot)); /* - * Reset datum's type ID fields to match the Var. + * Label the datum with the composite type info we identified before. */ - HeapTupleHeaderSetTypeId(dtuple, variable->vartype); - HeapTupleHeaderSetTypMod(dtuple, variable->vartypmod); - - ReleaseTupleDesc(var_tupdesc); + HeapTupleHeaderSetTypeId(dtuple, wrvstate->wrv_tupdesc->tdtypeid); + HeapTupleHeaderSetTypMod(dtuple, wrvstate->wrv_tupdesc->tdtypmod); return PointerGetDatum(dtuple); } @@ -2850,8 +2881,14 @@ ExecEvalConvertRowtype(ConvertRowtypeExprState *cstate, cstate->initialized = false; } - Assert(HeapTupleHeaderGetTypeId(tuple) == cstate->indesc->tdtypeid); - Assert(HeapTupleHeaderGetTypMod(tuple) == cstate->indesc->tdtypmod); + /* + * We used to be able to assert that incoming tuples are marked with + * exactly the rowtype of cstate->indesc. However, now that + * ExecEvalWholeRowVar might change the tuples' marking to plain RECORD + * due to inserting aliases, we can only make this weak test: + */ + Assert(HeapTupleHeaderGetTypeId(tuple) == cstate->indesc->tdtypeid || + HeapTupleHeaderGetTypeId(tuple) == RECORDOID); /* if first time through, initialize conversion map */ if (!cstate->initialized) @@ -4375,6 +4412,7 @@ ExecInitExpr(Expr *node, PlanState *parent) WholeRowVarExprState *wstate = makeNode(WholeRowVarExprState); wstate->parent = parent; + wstate->wrv_tupdesc = NULL; wstate->wrv_junkFilter = NULL; state = (ExprState *) wstate; state->evalfunc = (ExprStateEvalFunc) ExecEvalWholeRowVar; @@ -4778,17 +4816,18 @@ ExecInitExpr(Expr *node, PlanState *parent) /* Build tupdesc to describe result tuples */ if (rowexpr->row_typeid == RECORDOID) { - /* generic record, use runtime type assignment */ - rstate->tupdesc = ExecTypeFromExprList(rowexpr->args, - rowexpr->colnames); - BlessTupleDesc(rstate->tupdesc); - /* we won't need to redo this at runtime */ + /* generic record, use types of given expressions */ + rstate->tupdesc = ExecTypeFromExprList(rowexpr->args); } else { /* it's been cast to a named type, use that */ rstate->tupdesc = lookup_rowtype_tupdesc_copy(rowexpr->row_typeid, -1); } + /* In either case, adopt RowExpr's column aliases */ + ExecTypeSetColNames(rstate->tupdesc, rowexpr->colnames); + /* Bless the tupdesc in case it's now of type RECORD */ + BlessTupleDesc(rstate->tupdesc); /* Set up evaluation, skipping any deleted columns */ Assert(list_length(rowexpr->args) <= rstate->tupdesc->natts); attrs = rstate->tupdesc->attrs; diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 66515f71a2..4f5fcf6b69 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -950,28 +950,25 @@ ExecTypeFromTLInternal(List *targetList, bool hasoid, bool skipjunk) /* * ExecTypeFromExprList - build a tuple descriptor from a list of Exprs * - * Caller must also supply a list of field names (String nodes). + * This is roughly like ExecTypeFromTL, but we work from bare expressions + * not TargetEntrys. No names are attached to the tupledesc's columns. */ TupleDesc -ExecTypeFromExprList(List *exprList, List *namesList) +ExecTypeFromExprList(List *exprList) { TupleDesc typeInfo; - ListCell *le; - ListCell *ln; + ListCell *lc; int cur_resno = 1; - Assert(list_length(exprList) == list_length(namesList)); - typeInfo = CreateTemplateTupleDesc(list_length(exprList), false); - forboth(le, exprList, ln, namesList) + foreach(lc, exprList) { - Node *e = lfirst(le); - char *n = strVal(lfirst(ln)); + Node *e = lfirst(lc); TupleDescInitEntry(typeInfo, cur_resno, - n, + NULL, exprType(e), exprTypmod(e), 0); @@ -984,6 +981,54 @@ ExecTypeFromExprList(List *exprList, List *namesList) return typeInfo; } +/* + * ExecTypeSetColNames - set column names in a TupleDesc + * + * Column names must be provided as an alias list (list of String nodes). + * + * For some callers, the supplied tupdesc has a named rowtype (not RECORD) + * and it is moderately likely that the alias list matches the column names + * already present in the tupdesc. If we do change any column names then + * we must reset the tupdesc's type to anonymous RECORD; but we avoid doing + * so if no names change. + */ +void +ExecTypeSetColNames(TupleDesc typeInfo, List *namesList) +{ + bool modified = false; + int colno = 0; + ListCell *lc; + + foreach(lc, namesList) + { + char *cname = strVal(lfirst(lc)); + Form_pg_attribute attr; + + /* Guard against too-long names list */ + if (colno >= typeInfo->natts) + break; + attr = typeInfo->attrs[colno++]; + + /* Ignore empty aliases (these must be for dropped columns) */ + if (cname[0] == '\0') + continue; + + /* Change tupdesc only if alias is actually different */ + if (strcmp(cname, NameStr(attr->attname)) != 0) + { + namestrcpy(&(attr->attname), cname); + modified = true; + } + } + + /* If we modified the tupdesc, it's now a new record type */ + if (modified) + { + typeInfo->tdtypeid = RECORDOID; + typeInfo->tdtypmod = -1; + } +} + /* * BlessTupleDesc - make a completed tuple descriptor useful for SRFs * diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c index 945a414e96..4641708108 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); /* diff --git a/src/backend/executor/nodeValuesscan.c b/src/backend/executor/nodeValuesscan.c index 83b1324abc..d49a473386 100644 --- a/src/backend/executor/nodeValuesscan.c +++ b/src/backend/executor/nodeValuesscan.c @@ -25,7 +25,6 @@ #include "executor/executor.h" #include "executor/nodeValuesscan.h" -#include "parser/parsetree.h" static TupleTableSlot *ValuesNext(ValuesScanState *node); @@ -189,8 +188,6 @@ ValuesScanState * ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags) { ValuesScanState *scanstate; - RangeTblEntry *rte = rt_fetch(node->scan.scanrelid, - estate->es_range_table); TupleDesc tupdesc; ListCell *vtl; int i; @@ -242,8 +239,7 @@ ExecInitValuesScan(ValuesScan *node, EState *estate, int eflags) /* * get info about values list */ - tupdesc = ExecTypeFromExprList((List *) linitial(node->values_lists), - rte->eref->colnames); + tupdesc = ExecTypeFromExprList((List *) linitial(node->values_lists)); ExecAssignScanType(&scanstate->ss, tupdesc); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 02661350d9..87bb30039c 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -265,7 +265,8 @@ extern TupleTableSlot *ExecInitNullTupleSlot(EState *estate, TupleDesc tupType); extern TupleDesc ExecTypeFromTL(List *targetList, bool hasoid); extern TupleDesc ExecCleanTypeFromTL(List *targetList, bool hasoid); -extern TupleDesc ExecTypeFromExprList(List *exprList, List *namesList); +extern TupleDesc ExecTypeFromExprList(List *exprList); +extern void ExecTypeSetColNames(TupleDesc typeInfo, List *namesList); extern void UpdateChangedParamSet(PlanState *node, Bitmapset *newchg); typedef struct TupOutputState diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index fae281143f..d72439e2fa 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -577,6 +577,7 @@ typedef struct WholeRowVarExprState { ExprState xprstate; struct PlanState *parent; /* parent PlanState, or NULL if none */ + TupleDesc wrv_tupdesc; /* descriptor for resulting tuples */ JunkFilter *wrv_junkFilter; /* JunkFilter to remove resjunk cols */ } WholeRowVarExprState; diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index 88e7bfab84..54525de6b1 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -474,3 +474,163 @@ select (row('Jim', 'Beam')).text; -- error ERROR: could not identify column "text" in record data type LINE 1: select (row('Jim', 'Beam')).text; ^ +-- +-- Test that composite values are seen to have the correct column names +-- (bug #11210 and other reports) +-- +select row_to_json(i) from int8_tbl i; + row_to_json +------------------------------------------------ + {"q1":123,"q2":456} + {"q1":123,"q2":4567890123456789} + {"q1":4567890123456789,"q2":123} + {"q1":4567890123456789,"q2":4567890123456789} + {"q1":4567890123456789,"q2":-4567890123456789} +(5 rows) + +select row_to_json(i) from int8_tbl i(x,y); + row_to_json +---------------------------------------------- + {"x":123,"y":456} + {"x":123,"y":4567890123456789} + {"x":4567890123456789,"y":123} + {"x":4567890123456789,"y":4567890123456789} + {"x":4567890123456789,"y":-4567890123456789} +(5 rows) + +create temp view vv1 as select * from int8_tbl; +select row_to_json(i) from vv1 i; + row_to_json +------------------------------------------------ + {"q1":123,"q2":456} + {"q1":123,"q2":4567890123456789} + {"q1":4567890123456789,"q2":123} + {"q1":4567890123456789,"q2":4567890123456789} + {"q1":4567890123456789,"q2":-4567890123456789} +(5 rows) + +select row_to_json(i) from vv1 i(x,y); + row_to_json +---------------------------------------------- + {"x":123,"y":456} + {"x":123,"y":4567890123456789} + {"x":4567890123456789,"y":123} + {"x":4567890123456789,"y":4567890123456789} + {"x":4567890123456789,"y":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1, q2 from int8_tbl) as ss; + row_to_json +------------------------------------------------ + {"q1":123,"q2":456} + {"q1":123,"q2":4567890123456789} + {"q1":4567890123456789,"q2":123} + {"q1":4567890123456789,"q2":4567890123456789} + {"q1":4567890123456789,"q2":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1, q2 from int8_tbl offset 0) as ss; + row_to_json +------------------------------------------------ + {"q1":123,"q2":456} + {"q1":123,"q2":4567890123456789} + {"q1":4567890123456789,"q2":123} + {"q1":4567890123456789,"q2":4567890123456789} + {"q1":4567890123456789,"q2":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl) as ss; + row_to_json +---------------------------------------------- + {"a":123,"b":456} + {"a":123,"b":4567890123456789} + {"a":4567890123456789,"b":123} + {"a":4567890123456789,"b":4567890123456789} + {"a":4567890123456789,"b":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl offset 0) as ss; + row_to_json +---------------------------------------------- + {"a":123,"b":456} + {"a":123,"b":4567890123456789} + {"a":4567890123456789,"b":123} + {"a":4567890123456789,"b":4567890123456789} + {"a":4567890123456789,"b":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl) as ss(x,y); + row_to_json +---------------------------------------------- + {"x":123,"y":456} + {"x":123,"y":4567890123456789} + {"x":4567890123456789,"y":123} + {"x":4567890123456789,"y":4567890123456789} + {"x":4567890123456789,"y":-4567890123456789} +(5 rows) + +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl offset 0) as ss(x,y); + row_to_json +---------------------------------------------- + {"x":123,"y":456} + {"x":123,"y":4567890123456789} + {"x":4567890123456789,"y":123} + {"x":4567890123456789,"y":4567890123456789} + {"x":4567890123456789,"y":-4567890123456789} +(5 rows) + +explain (costs off) +select row_to_json(q) from + (select thousand, tenthous from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; + QUERY PLAN +------------------------------------------------------------- + Subquery Scan on q + -> Index Only Scan using tenk1_thous_tenthous on tenk1 + Index Cond: ((thousand = 42) AND (tenthous < 2000)) +(3 rows) + +select row_to_json(q) from + (select thousand, tenthous from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; + row_to_json +--------------------------------- + {"thousand":42,"tenthous":42} + {"thousand":42,"tenthous":1042} +(2 rows) + +select row_to_json(q) from + (select thousand as x, tenthous as y from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; + row_to_json +------------------- + {"x":42,"y":42} + {"x":42,"y":1042} +(2 rows) + +select row_to_json(q) from + (select thousand as x, tenthous as y from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q(a,b); + row_to_json +------------------- + {"a":42,"b":42} + {"a":42,"b":1042} +(2 rows) + +create temp table tt1 as select * from int8_tbl limit 2; +create temp table tt2 () inherits(tt1); +insert into tt2 values(0,0); +select row_to_json(r) from (select q2,q1 from tt1 offset 0) r; + row_to_json +---------------------------------- + {"q2":456,"q1":123} + {"q2":4567890123456789,"q1":123} + {"q2":0,"q1":0} +(3 rows) + diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 65ebdc566a..bc3f021020 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -227,3 +227,47 @@ select cast (row('Jim', 'Beam') as text); select (row('Jim', 'Beam'))::text; select text(row('Jim', 'Beam')); -- error select (row('Jim', 'Beam')).text; -- error + +-- +-- Test that composite values are seen to have the correct column names +-- (bug #11210 and other reports) +-- + +select row_to_json(i) from int8_tbl i; +select row_to_json(i) from int8_tbl i(x,y); + +create temp view vv1 as select * from int8_tbl; +select row_to_json(i) from vv1 i; +select row_to_json(i) from vv1 i(x,y); + +select row_to_json(ss) from + (select q1, q2 from int8_tbl) as ss; +select row_to_json(ss) from + (select q1, q2 from int8_tbl offset 0) as ss; +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl) as ss; +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl offset 0) as ss; +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl) as ss(x,y); +select row_to_json(ss) from + (select q1 as a, q2 as b from int8_tbl offset 0) as ss(x,y); + +explain (costs off) +select row_to_json(q) from + (select thousand, tenthous from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; +select row_to_json(q) from + (select thousand, tenthous from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; +select row_to_json(q) from + (select thousand as x, tenthous as y from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q; +select row_to_json(q) from + (select thousand as x, tenthous as y from tenk1 + where thousand = 42 and tenthous < 2000 offset 0) q(a,b); + +create temp table tt1 as select * from int8_tbl limit 2; +create temp table tt2 () inherits(tt1); +insert into tt2 values(0,0); +select row_to_json(r) from (select q2,q1 from tt1 offset 0) r;