From 292176a118da6979e5d368a4baf27f26896c99a5 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 11 Jan 2010 15:31:04 +0000 Subject: [PATCH] Improve ExecEvalVar's handling of whole-row variables in cases where the rowtype contains dropped columns. Sometimes the input tuple will be formed from a select targetlist in which dropped columns are filled with a NULL of an arbitrary type (the planner typically uses INT4, since it can't tell what type the dropped column really was). So we need to relax the rowtype compatibility check to not insist on physical compatibility if the actual column value is NULL. In principle we might need to do this for functions returning composite types, too (see tupledesc_match()). In practice there doesn't seem to be a bug there, probably because the function will be using the same cached rowtype descriptor as the caller. Fixing that code path would require significant rearrangement, so I left it alone for now. Per complaint from Filip Rembialkowski. --- src/backend/executor/execQual.c | 74 +++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index f96eeccd06..4462a29332 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.260 2010/01/09 20:46:19 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.261 2010/01/11 15:31:04 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -591,17 +591,23 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext, /* * We really only care about number of attributes and data type. * Also, we can ignore type mismatch on columns that are dropped - * in the destination type, so long as the physical storage - * matches. This is helpful in some cases involving out-of-date - * cached plans. Also, we have to allow the case that the slot - * has more columns than the Var's type, because we might be - * looking at the output of a subplan that includes resjunk - * columns. (XXX it would be nice to verify that the extra - * columns are all marked resjunk, but we haven't got access to - * the subplan targetlist here...) Resjunk columns should always - * be at the end of a targetlist, so it's sufficient to ignore - * them here; but we need to use ExecEvalWholeRowSlow to get rid - * of them in the eventual output tuples. + * in the destination type, so long as (1) the physical storage + * matches or (2) the actual column value is NULL. Case (1) is + * helpful in some cases involving out-of-date cached plans, while + * case (2) is expected behavior in situations such as an INSERT + * into a table with dropped columns (the planner typically + * generates an INT4 NULL regardless of the dropped column type). + * If we find a dropped column and cannot verify that case (1) + * holds, we have to use ExecEvalWholeRowSlow to check (2) for + * each row. Also, we have to allow the case that the slot has + * more columns than the Var's type, because we might be looking + * at the output of a subplan that includes resjunk columns. + * (XXX it would be nice to verify that the extra columns are all + * marked resjunk, but we haven't got access to the subplan + * targetlist here...) Resjunk columns should always be at the end + * of a targetlist, so it's sufficient to ignore them here; but we + * need to use ExecEvalWholeRowSlow to get rid of them in the + * eventual output tuples. */ var_tupdesc = lookup_rowtype_tupdesc(variable->vartype, -1); @@ -615,7 +621,7 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext, slot_tupdesc->natts, var_tupdesc->natts))); else if (var_tupdesc->natts < slot_tupdesc->natts) - needslow = true; + needslow = true; /* need to trim trailing atts */ for (i = 0; i < var_tupdesc->natts; i++) { @@ -635,11 +641,7 @@ ExecEvalVar(ExprState *exprstate, ExprContext *econtext, if (vattr->attlen != sattr->attlen || vattr->attalign != sattr->attalign) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("table row type and query-specified row type do not match"), - errdetail("Physical storage mismatch on dropped attribute at ordinal position %d.", - i + 1))); + needslow = true; /* need runtime check for null */ } ReleaseTupleDesc(var_tupdesc); @@ -766,7 +768,7 @@ ExecEvalWholeRowVar(ExprState *exprstate, ExprContext *econtext, /* ---------------------------------------------------------------- * ExecEvalWholeRowSlow * - * Returns a Datum for a whole-row variable, in the "slow" case where + * Returns a Datum for a whole-row variable, in the "slow" cases where * we can't just copy the subplan's output. * ---------------------------------------------------------------- */ @@ -779,6 +781,7 @@ ExecEvalWholeRowSlow(ExprState *exprstate, ExprContext *econtext, HeapTuple tuple; TupleDesc var_tupdesc; HeapTupleHeader dtuple; + int i; if (isDone) *isDone = ExprSingleResult; @@ -802,18 +805,38 @@ ExecEvalWholeRowSlow(ExprState *exprstate, ExprContext *econtext, } /* - * Currently, the only case handled here is stripping of trailing resjunk - * fields, which we do in a slightly chintzy way by just adjusting the - * tuple's natts header field. Possibly there will someday be a need for - * more-extensive rearrangements, in which case it'd be worth - * disassembling and reassembling the tuple (perhaps use a JunkFilter for - * that?) + * Currently, the only data modification case handled here is stripping of + * trailing resjunk fields, which we do in a slightly chintzy way by just + * adjusting the tuple's natts header field. Possibly there will someday + * be a need for more-extensive rearrangements, in which case we'd + * probably use tupconvert.c. */ Assert(variable->vartype != RECORDOID); var_tupdesc = lookup_rowtype_tupdesc(variable->vartype, -1); tuple = ExecFetchSlotTuple(slot); + Assert(HeapTupleHeaderGetNatts(tuple->t_data) >= var_tupdesc->natts); + + /* Check to see if any dropped attributes are non-null */ + for (i = 0; i < var_tupdesc->natts; i++) + { + Form_pg_attribute vattr = var_tupdesc->attrs[i]; + Form_pg_attribute sattr = slot->tts_tupleDescriptor->attrs[i]; + + if (!vattr->attisdropped) + continue; /* already checked non-dropped cols */ + if (heap_attisnull(tuple, i+1)) + continue; /* null is always okay */ + if (vattr->attlen != sattr->attlen || + vattr->attalign != sattr->attalign) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("table row type and query-specified row type do not match"), + errdetail("Physical storage mismatch on dropped attribute at ordinal position %d.", + i + 1))); + } + /* * We have to make a copy of the tuple so we can safely insert the Datum * overhead fields, which are not set in on-disk tuples; not to mention @@ -826,7 +849,6 @@ ExecEvalWholeRowSlow(ExprState *exprstate, ExprContext *econtext, HeapTupleHeaderSetTypeId(dtuple, variable->vartype); HeapTupleHeaderSetTypMod(dtuple, variable->vartypmod); - Assert(HeapTupleHeaderGetNatts(dtuple) >= var_tupdesc->natts); HeapTupleHeaderSetNatts(dtuple, var_tupdesc->natts); ReleaseTupleDesc(var_tupdesc); -- 2.40.0