From 7c03cd1156b5e1ecbb5c063a22fba3a047a02c24 Mon Sep 17 00:00:00 2001 From: Nikita Malakhov Date: Thu, 9 Jul 2026 16:21:32 +0300 Subject: [PATCH] DELETE/UPDATE more than one row in partitioned foreign table (1/2) There is known bug in delete and update rows in partitioned foreign tables via FDW [1] Current copyTuple and copySlot machinery supposes that source and destination slots and tuples always match. This patch modifies copy internals to consider more complex case and copy only actual attributes, and if destination slot/tuple has more they are set to null. This is a proof of concept patch which shows such mechanics does not break anything and allows to use it in cases when we have to pass or receive not matching number of attributes. Core patch - introduction of remote table OID returned from remote query and passing it through planner and executor to use in foreign queries [1] Discussion: https://www.postgresql.org/message-id/flat/20250718175314.4513c00a%40karst --- src/backend/access/common/heaptuple.c | 127 ++++++++++++++++++++++-- src/backend/executor/execGrouping.c | 3 +- src/backend/executor/execMain.c | 4 + src/backend/executor/execTuples.c | 107 ++++++++++++++------ src/backend/executor/nodeForeignscan.c | 2 + src/backend/optimizer/path/allpaths.c | 20 ++++ src/backend/optimizer/plan/createplan.c | 19 ++++ src/backend/optimizer/plan/initsplan.c | 45 +++++++++ src/backend/optimizer/plan/planner.c | 14 ++- src/backend/optimizer/plan/setrefs.c | 56 +++++++++++ src/backend/optimizer/plan/subselect.c | 7 +- src/backend/optimizer/util/appendinfo.c | 27 +++++ src/backend/optimizer/util/inherit.c | 1 + src/backend/optimizer/util/relnode.c | 21 ++++ src/backend/optimizer/util/tlist.c | 39 ++++++-- src/backend/utils/adt/ruleutils.c | 2 +- src/include/access/htup_details.h | 22 ++++ src/include/executor/tuptable.h | 42 ++++++-- src/include/nodes/pathnodes.h | 7 ++ src/include/nodes/primnodes.h | 1 + 20 files changed, 500 insertions(+), 66 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index f30346469ed..d4fbd962ec1 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -61,6 +61,7 @@ #include "access/sysattr.h" #include "access/tupdesc_details.h" #include "common/hashfn.h" +#include "executor/tuptable.h" #include "utils/datum.h" #include "utils/expandeddatum.h" #include "utils/hsearch.h" @@ -219,11 +220,30 @@ Size heap_compute_data_size(TupleDesc tupleDesc, const Datum *values, const bool *isnull) +{ + return heap_compute_data_size_ext(tupleDesc, values, isnull, + - 1 /* take natts from tupleDesc */); +} + +/* + * heap_compute_data_size_ext + * Same as above, but allows to specify number of attributes explicitly. + * + * If "natts" <= 0, then number of attributes is taken from the TupleDesc. + */ +Size +heap_compute_data_size_ext(TupleDesc tupleDesc, + const Datum *values, + const bool *isnull, + int natts) { Size data_length = 0; int i; int numberOfAttributes = tupleDesc->natts; + if (natts > 0) + numberOfAttributes = natts; + for (i = 0; i < numberOfAttributes; i++) { Datum val; @@ -402,6 +422,22 @@ heap_fill_tuple(TupleDesc tupleDesc, const Datum *values, const bool *isnull, char *data, Size data_size, uint16 *infomask, uint8 *bit) +{ + heap_fill_tuple_ext(tupleDesc, values, isnull, data, data_size, infomask, + bit, -1 /* take natts from tupleDesc */); +} + +/* + * heap_fill_tuple_ext + * Same as above, but allows to specify number of attributes explicitly. + * + * If "natts" <= 0, then number of attributes is taken from the TupleDesc. + */ +void +heap_fill_tuple_ext(TupleDesc tupleDesc, + const Datum *values, const bool *isnull, + char *data, Size data_size, + uint16 *infomask, uint8 *bit, int natts) { uint8 *bitP; int bitmask; @@ -412,6 +448,9 @@ heap_fill_tuple(TupleDesc tupleDesc, char *start = data; #endif + if (natts > 0) + numberOfAttributes = natts; + if (bit != NULL) { bitP = &bit[-1]; @@ -684,18 +723,48 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull) */ HeapTuple heap_copytuple(HeapTuple tuple) +{ + return heap_copytuple_ext(tuple, NULL, -1); +} + +/* ---------------- + * heap_copytuple_ext + * + * returns a copy of a tuple consists of attnum atts of original + * + * The HeapTuple struct, tuple header, and tuple data are all allocated + * as a single palloc() block. + * ---------------- + */ +HeapTuple +heap_copytuple_ext(HeapTuple tuple, TupleTableSlot *slot, int dstnatts) { HeapTuple newTuple; + Size tuplen; if (!HeapTupleIsValid(tuple) || tuple->t_data == NULL) return NULL; - newTuple = (HeapTuple) palloc(HEAPTUPLESIZE + tuple->t_len); - newTuple->t_len = tuple->t_len; + tuplen = tuple->t_len; + + if (slot != NULL) + { + if (dstnatts < 0) + dstnatts = slot->tts_tupleDescriptor->natts; + + if (dstnatts < slot->tts_tupleDescriptor->natts) + tuplen = heap_compute_data_size_ext(slot->tts_tupleDescriptor, + slot->tts_values, + slot->tts_isnull, + dstnatts); + } + + newTuple = (HeapTuple) palloc(HEAPTUPLESIZE + tuplen); + newTuple->t_len = tuplen; newTuple->t_self = tuple->t_self; newTuple->t_tableOid = tuple->t_tableOid; newTuple->t_data = (HeapTupleHeader) ((char *) newTuple + HEAPTUPLESIZE); - memcpy(newTuple->t_data, tuple->t_data, tuple->t_len); + memcpy(newTuple->t_data, tuple->t_data, tuplen); return newTuple; } @@ -1025,6 +1094,21 @@ HeapTuple heap_form_tuple(TupleDesc tupleDescriptor, const Datum *values, const bool *isnull) +{ + return heap_form_tuple_ext(tupleDescriptor, values, isnull, -1); +} + +/* + * heap_form_tuple_ext + * Same as above, but allows to specify number of attributes explicitly. + * + * If "natts" <= 0, then number of attributes is taken from the TupleDesc. + */ +HeapTuple +heap_form_tuple_ext(TupleDesc tupleDescriptor, + const Datum *values, + const bool *isnull, + int natts) { HeapTuple tuple; /* return tuple */ HeapTupleHeader td; /* tuple data */ @@ -1035,6 +1119,9 @@ heap_form_tuple(TupleDesc tupleDescriptor, int numberOfAttributes = tupleDescriptor->natts; int i; + if(natts > 0) + numberOfAttributes = natts; + if (numberOfAttributes > MaxTupleAttributeNumber) ereport(ERROR, (errcode(ERRCODE_TOO_MANY_COLUMNS), @@ -1063,7 +1150,7 @@ heap_form_tuple(TupleDesc tupleDescriptor, hoff = len = MAXALIGN(len); /* align user data safely */ - data_len = heap_compute_data_size(tupleDescriptor, values, isnull); + data_len = heap_compute_data_size_ext(tupleDescriptor, values, isnull, natts); len += data_len; @@ -1092,13 +1179,14 @@ heap_form_tuple(TupleDesc tupleDescriptor, HeapTupleHeaderSetNatts(td, numberOfAttributes); td->t_hoff = hoff; - heap_fill_tuple(tupleDescriptor, + heap_fill_tuple_ext(tupleDescriptor, values, isnull, (char *) td + hoff, data_len, &td->t_infomask, - (hasnull ? td->t_bits : NULL)); + (hasnull ? td->t_bits : NULL), + natts); return tuple; } @@ -1391,6 +1479,23 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor, const Datum *values, const bool *isnull, Size extra) +{ + return heap_form_minimal_tuple_ext(tupleDescriptor, values, isnull, extra, + -1 /* take natts from tupleDesc */); +} + +/* + * heap_form_minimal_tuple_ext + * Same as above, but allows to specify number of attributes explicitly. + * + * If "natts" <= 0, then number of attributes is taken from the TupleDesc. + */ +MinimalTuple +heap_form_minimal_tuple_ext(TupleDesc tupleDescriptor, + const Datum *values, + const bool *isnull, + Size extra, + int natts) { MinimalTuple tuple; /* return tuple */ char *mem; @@ -1401,6 +1506,9 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor, int numberOfAttributes = tupleDescriptor->natts; int i; + if (natts > 0) + numberOfAttributes = natts; + Assert(extra == MAXALIGN(extra)); if (numberOfAttributes > MaxTupleAttributeNumber) @@ -1431,7 +1539,7 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor, hoff = len = MAXALIGN(len); /* align user data safely */ - data_len = heap_compute_data_size(tupleDescriptor, values, isnull); + data_len = heap_compute_data_size_ext(tupleDescriptor, values, isnull, natts); len += data_len; @@ -1448,13 +1556,14 @@ heap_form_minimal_tuple(TupleDesc tupleDescriptor, HeapTupleHeaderSetNatts(tuple, numberOfAttributes); tuple->t_hoff = hoff + MINIMAL_TUPLE_OFFSET; - heap_fill_tuple(tupleDescriptor, + heap_fill_tuple_ext(tupleDescriptor, values, isnull, (char *) tuple + hoff, data_len, &tuple->t_infomask, - (hasnull ? tuple->t_bits : NULL)); + (hasnull ? tuple->t_bits : NULL), + natts); return tuple; } diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index c107514a85d..7ef7c8c6d9d 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -583,7 +583,8 @@ LookupTupleHashEntry_internal(TupleHashTable hashtable, TupleTableSlot *slot, * TupleHashEntryData or allocate an additional chunk. */ entry->firstTuple = ExecCopySlotMinimalTupleExtra(slot, - hashtable->additionalsize); + hashtable->additionalsize, + -1 /* natts */); } } else diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index fde502efd38..5bd766a3a81 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2658,6 +2658,10 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist) resname); if (!AttributeNumberIsValid(aerm->ctidAttNo)) elog(ERROR, "could not find junk %s column", resname); +//fdwproblem + snprintf(resname, sizeof(resname), "tableoid%u", erm->rowmarkId); + aerm->toidAttNo = ExecFindJunkAttributeInTlist(targetlist, + resname); } else { diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 7f4ebf95432..5462d26532a 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -79,6 +79,9 @@ static inline void tts_buffer_heap_store_tuple(TupleTableSlot *slot, Buffer buffer, bool transfer_pin); static void tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree); +static void tts_heap_materialize_ext(TupleTableSlot *slot, int natts); +static void tts_minimal_materialize_ext(TupleTableSlot *slot, int natts); +static void tts_buffer_heap_materialize_ext(TupleTableSlot *slot, int natts); const TupleTableSlotOps TTSOpsVirtual; @@ -269,18 +272,29 @@ static void tts_virtual_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) { TupleDesc srcdesc = srcslot->tts_tupleDescriptor; + TupleDesc dstdesc = dstslot->tts_tupleDescriptor; + int min_natts = Min(dstdesc->natts, srcdesc->natts); tts_virtual_clear(dstslot); slot_getallattrs(srcslot); - for (int natt = 0; natt < srcdesc->natts; natt++) + for (int natt = 0; natt < min_natts; natt++) { dstslot->tts_values[natt] = srcslot->tts_values[natt]; dstslot->tts_isnull[natt] = srcslot->tts_isnull[natt]; } - dstslot->tts_nvalid = srcdesc->natts; + if (dstdesc->natts > srcdesc->natts) + { + for (int natt = min_natts; natt < dstdesc->natts; natt++) + { + dstslot->tts_values[natt] = (Datum) 0; + dstslot->tts_isnull[natt] = true; + } + } + + dstslot->tts_nvalid = dstdesc->natts; dstslot->tts_flags &= ~TTS_FLAG_EMPTY; /* make sure storage doesn't depend on external memory */ @@ -288,24 +302,26 @@ tts_virtual_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) } static HeapTuple -tts_virtual_copy_heap_tuple(TupleTableSlot *slot) +tts_virtual_copy_heap_tuple(TupleTableSlot *slot, int natts) { Assert(!TTS_EMPTY(slot)); - return heap_form_tuple(slot->tts_tupleDescriptor, + return heap_form_tuple_ext(slot->tts_tupleDescriptor, slot->tts_values, - slot->tts_isnull); + slot->tts_isnull, + natts); } static MinimalTuple -tts_virtual_copy_minimal_tuple(TupleTableSlot *slot, Size extra) +tts_virtual_copy_minimal_tuple(TupleTableSlot *slot, Size extra, int natts) { Assert(!TTS_EMPTY(slot)); - return heap_form_minimal_tuple(slot->tts_tupleDescriptor, + return heap_form_minimal_tuple_ext(slot->tts_tupleDescriptor, slot->tts_values, slot->tts_isnull, - extra); + extra, + natts); } @@ -397,6 +413,12 @@ tts_heap_is_current_xact_tuple(TupleTableSlot *slot) static void tts_heap_materialize(TupleTableSlot *slot) +{ + tts_heap_materialize_ext(slot, -1); +} + +static void +tts_heap_materialize_ext(TupleTableSlot *slot, int natts) { HeapTupleTableSlot *hslot = (HeapTupleTableSlot *) slot; MemoryContext oldContext; @@ -417,9 +439,10 @@ tts_heap_materialize(TupleTableSlot *slot) hslot->off = 0; if (!hslot->tuple) - hslot->tuple = heap_form_tuple(slot->tts_tupleDescriptor, + hslot->tuple = heap_form_tuple_ext(slot->tts_tupleDescriptor, slot->tts_values, - slot->tts_isnull); + slot->tts_isnull, + natts); else { /* @@ -427,7 +450,7 @@ tts_heap_materialize(TupleTableSlot *slot) * context of the given slot (else it would have TTS_FLAG_SHOULDFREE * set). Copy the tuple into the given slot's memory context. */ - hslot->tuple = heap_copytuple(hslot->tuple); + hslot->tuple = heap_copytuple_ext(hslot->tuple, slot, natts); } slot->tts_flags |= TTS_FLAG_SHOULDFREE; @@ -461,24 +484,24 @@ tts_heap_get_heap_tuple(TupleTableSlot *slot) } static HeapTuple -tts_heap_copy_heap_tuple(TupleTableSlot *slot) +tts_heap_copy_heap_tuple(TupleTableSlot *slot, int natts) { HeapTupleTableSlot *hslot = (HeapTupleTableSlot *) slot; Assert(!TTS_EMPTY(slot)); if (!hslot->tuple) - tts_heap_materialize(slot); + tts_heap_materialize_ext(slot, natts); - return heap_copytuple(hslot->tuple); + return heap_copytuple_ext(hslot->tuple, slot, natts); } static MinimalTuple -tts_heap_copy_minimal_tuple(TupleTableSlot *slot, Size extra) +tts_heap_copy_minimal_tuple(TupleTableSlot *slot, Size extra, int natts) { HeapTupleTableSlot *hslot = (HeapTupleTableSlot *) slot; if (!hslot->tuple) - tts_heap_materialize(slot); + tts_heap_materialize_ext(slot, natts); return minimal_tuple_from_heap_tuple(hslot->tuple, extra); } @@ -585,6 +608,12 @@ tts_minimal_is_current_xact_tuple(TupleTableSlot *slot) static void tts_minimal_materialize(TupleTableSlot *slot) +{ + tts_minimal_materialize_ext(slot, -1); +} + +static void +tts_minimal_materialize_ext(TupleTableSlot *slot, int natts) { MinimalTupleTableSlot *mslot = (MinimalTupleTableSlot *) slot; MemoryContext oldContext; @@ -606,10 +635,11 @@ tts_minimal_materialize(TupleTableSlot *slot) if (!mslot->mintuple) { - mslot->mintuple = heap_form_minimal_tuple(slot->tts_tupleDescriptor, + mslot->mintuple = heap_form_minimal_tuple_ext(slot->tts_tupleDescriptor, slot->tts_values, slot->tts_isnull, - 0); + 0 /* no extra */, + natts); } else { @@ -637,9 +667,13 @@ tts_minimal_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) { MemoryContext oldcontext; MinimalTuple mintuple; + int min_natts; + + min_natts = Min(dstslot->tts_tupleDescriptor->natts, + srcslot->tts_tupleDescriptor->natts); oldcontext = MemoryContextSwitchTo(dstslot->tts_mcxt); - mintuple = ExecCopySlotMinimalTuple(srcslot); + mintuple = ExecCopySlotMinimalTupleExteded(srcslot, min_natts); MemoryContextSwitchTo(oldcontext); ExecStoreMinimalTuple(mintuple, dstslot, true); @@ -657,7 +691,7 @@ tts_minimal_get_minimal_tuple(TupleTableSlot *slot) } static HeapTuple -tts_minimal_copy_heap_tuple(TupleTableSlot *slot) +tts_minimal_copy_heap_tuple(TupleTableSlot *slot, int natts) { MinimalTupleTableSlot *mslot = (MinimalTupleTableSlot *) slot; @@ -668,7 +702,7 @@ tts_minimal_copy_heap_tuple(TupleTableSlot *slot) } static MinimalTuple -tts_minimal_copy_minimal_tuple(TupleTableSlot *slot, Size extra) +tts_minimal_copy_minimal_tuple(TupleTableSlot *slot, Size extra, int natts) { MinimalTupleTableSlot *mslot = (MinimalTupleTableSlot *) slot; @@ -802,6 +836,12 @@ tts_buffer_is_current_xact_tuple(TupleTableSlot *slot) static void tts_buffer_heap_materialize(TupleTableSlot *slot) +{ + tts_buffer_heap_materialize_ext(slot, -1 /* natts */); +} + +static void +tts_buffer_heap_materialize_ext(TupleTableSlot *slot, int natts) { BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot; MemoryContext oldContext; @@ -874,12 +914,16 @@ tts_buffer_heap_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) TTS_SHOULDFREE(srcslot) || !bsrcslot->base.tuple) { + int min_natts; MemoryContext oldContext; + min_natts = Min(dstslot->tts_tupleDescriptor->natts, + srcslot->tts_tupleDescriptor->natts); + ExecClearTuple(dstslot); dstslot->tts_flags &= ~TTS_FLAG_EMPTY; oldContext = MemoryContextSwitchTo(dstslot->tts_mcxt); - bdstslot->base.tuple = ExecCopySlotHeapTuple(srcslot); + bdstslot->base.tuple = ExecCopySlotHeapTupleExteded(srcslot, min_natts); dstslot->tts_flags |= TTS_FLAG_SHOULDFREE; MemoryContextSwitchTo(oldContext); } @@ -915,27 +959,27 @@ tts_buffer_heap_get_heap_tuple(TupleTableSlot *slot) } static HeapTuple -tts_buffer_heap_copy_heap_tuple(TupleTableSlot *slot) +tts_buffer_heap_copy_heap_tuple(TupleTableSlot *slot, int natts) { BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot; Assert(!TTS_EMPTY(slot)); if (!bslot->base.tuple) - tts_buffer_heap_materialize(slot); + tts_buffer_heap_materialize_ext(slot, natts); - return heap_copytuple(bslot->base.tuple); + return heap_copytuple_ext(bslot->base.tuple, slot, natts); } static MinimalTuple -tts_buffer_heap_copy_minimal_tuple(TupleTableSlot *slot, Size extra) +tts_buffer_heap_copy_minimal_tuple(TupleTableSlot *slot, Size extra, int natts) { BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot; Assert(!TTS_EMPTY(slot)); if (!bslot->base.tuple) - tts_buffer_heap_materialize(slot); + tts_buffer_heap_materialize_ext(slot, natts); return minimal_tuple_from_heap_tuple(bslot->base.tuple, extra); } @@ -1874,8 +1918,7 @@ ExecStoreAllNullTuple(TupleTableSlot *slot) * Until the slot is materialized, the contents of the slot depend on the * datum. */ -void -ExecStoreHeapTupleDatum(Datum data, TupleTableSlot *slot) +void ExecStoreHeapTupleDatum(Datum data, TupleTableSlot *slot) { HeapTupleData tuple = {0}; HeapTupleHeader td; @@ -1885,6 +1928,7 @@ ExecStoreHeapTupleDatum(Datum data, TupleTableSlot *slot) tuple.t_len = HeapTupleHeaderGetDatumLength(td); tuple.t_self = td->t_ctid; tuple.t_data = td; + tuple.t_tableOid = InvalidOid; ExecClearTuple(slot); @@ -1929,7 +1973,7 @@ ExecFetchSlotHeapTuple(TupleTableSlot *slot, bool materialize, bool *shouldFree) { if (shouldFree) *shouldFree = true; - return slot->tts_ops->copy_heap_tuple(slot); + return slot->tts_ops->copy_heap_tuple(slot, -1 /* natts */); } else { @@ -1980,7 +2024,8 @@ ExecFetchSlotMinimalTuple(TupleTableSlot *slot, { if (shouldFree) *shouldFree = true; - return slot->tts_ops->copy_minimal_tuple(slot, 0); + return slot->tts_ops->copy_minimal_tuple(slot, 0 /* no extra */, + -1 /* natts */); } } diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 6f0daddce07..21b3e9a961b 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -65,6 +65,8 @@ ForeignNext(ForeignScanState *node) * Insert valid value into tableoid, the only actually-useful system * column. */ + + slot->tts_remoteOid = slot->tts_tableOid; if (plan->fsSystemCol && !TupIsNull(slot)) slot->tts_tableOid = RelationGetRelid(node->ss.ss_currentRelation); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 9c040ecefc8..eb71d2bb54a 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -1165,6 +1165,26 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, (Node *) rel->reltarget->exprs, 1, &appinfo); + /* Do it if fdw is partition */ + if (planner_rt_fetch(childRTindex, root)->relkind == RELKIND_FOREIGN_TABLE && + !bms_is_empty(root->glob->foreignParamIDs)) + { + foreach(lc, root->processed_tlist) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + Param *param = (Param *) tle->expr; + + if (tle->resjunk && IsA(param, Param) && + IS_FOREIGN_PARAM(root, param) && + param->target_rte == childRTindex) // TODO same for another case + { + /* XXX is copyObject necessary here? */ + childrel->reltarget->exprs = + lappend(childrel->reltarget->exprs, copyObject(param)); + } + } + } + /* * We have to make child entries in the EquivalenceClass data * structures as well. This is needed either if the parent diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 2c696ea0268..ca9226391a5 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -986,6 +986,25 @@ use_physical_tlist(PlannerInfo *root, Path *path, int flags) } } + /* + * Also, can't do it to a ForeignPath if the path is requested to emit + * Params generated by the FDW. + */ + if (IsA(path, ForeignPath) && + path->parent->relid == root->parse->resultRelation && + !bms_is_empty(root->glob->foreignParamIDs)) + { + foreach(lc, path->pathtarget->exprs) + { + Param *param = (Param *) lfirst(lc); + if (param && IsA(param, Param)) + { + Assert(IS_FOREIGN_PARAM(root, param)); + return false; + } + } + } + return true; } diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index b38422c47a4..cc09065e561 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -32,6 +32,7 @@ #include "optimizer/planner.h" #include "optimizer/restrictinfo.h" #include "parser/analyze.h" +#include "parser/parsetree.h" #include "rewrite/rewriteManip.h" #include "utils/lsyscache.h" #include "utils/rel.h" @@ -243,6 +244,40 @@ add_other_rels_to_query(PlannerInfo *root) * *****************************************************************************/ +/* + * add_params_to_result_rel + * If the query's final tlist contains Params the FDW generated, add + * targetlist entries for each such Param to the result relation. + */ +static void +add_params_to_result_rel(PlannerInfo *root, List *final_tlist) +{ + RelOptInfo *target_rel = find_base_rel(root, root->parse->resultRelation); + ListCell *lc; + + /* + * If no parameters have been generated by any FDWs, we certainly don't + * need to do anything here. + */ + if (bms_is_empty(root->glob->foreignParamIDs)) + return; + + foreach(lc, final_tlist) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + Param *param = (Param *) tle->expr; + + if (tle->resjunk && IsA(param, Param) && + IS_FOREIGN_PARAM(root, param) && + param->target_rte == target_rel->relid) + { + /* XXX is copyObject necessary here? */ + target_rel->reltarget->exprs = lappend(target_rel->reltarget->exprs, + copyObject(param)); + } + } +} + /* * build_base_rel_tlists * Add targetlist entries for each var needed in the query's final tlist @@ -282,6 +317,16 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist) list_free(having_vars); } } + + if (root->parse->commandType == CMD_UPDATE || + root->parse->commandType == CMD_DELETE) + { + int result_relation = root->parse->resultRelation; + + if (planner_rt_fetch(result_relation, root)->relkind == RELKIND_FOREIGN_TABLE) + add_params_to_result_rel(root, final_tlist); + + } } /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 3225185d16f..03439053355 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -388,6 +388,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, glob->dependsOnRole = false; glob->partition_directory = NULL; glob->rel_notnullatts_hash = NULL; + glob->foreignParamIDs = NULL; /* * Assess whether it's feasible to use parallel mode for this query. We @@ -616,12 +617,15 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions, } /* - * If any Params were generated, run through the plan tree and compute - * each plan node's extParam/allParam sets. Ideally we'd merge this into - * set_plan_references' tree traversal, but for now it has to be separate - * because we need to visit subplans before not after main plan. + * If any Params were generated by the planner not by FDWs, run through + * the plan tree and compute each plan node's extParam/allParam sets. + * (Params added by FDWs are irrelevant for parameter change signaling.) + * Ideally we'd merge this into set_plan_references' tree traversal, but + * for now it has to be separate because we need to visit subplans before + * not after main plan. */ - if (glob->paramExecTypes != NIL) + if (glob->paramExecTypes != NIL && + bms_num_members(glob->foreignParamIDs) < list_length(glob->paramExecTypes)) { Assert(list_length(glob->subplans) == list_length(glob->subroots)); forboth(lp, glob->subplans, lr, glob->subroots) diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index e1182255055..bf8f8fdab4c 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -3277,7 +3277,42 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context) } /* Special cases (apply only AFTER failing to match to lower tlist) */ if (IsA(node, Param)) + { + Param *param = (Param *) node; + + /* + * If the Param is a PARAM_EXEC Param generated by an FDW, it should + * have bubbled up from a lower plan node; convert it into a simple + * Var referencing the output of the subplan. + * + * Note: set_join_references() would have kept has_non_vars=true for + * the subplan emitting the Param since it effectively belong to the + * result relation and that relation can never be the nullable side of + * an outer join. + */ + if (IS_FOREIGN_PARAM(context->root, param)) + { + if (context->outer_itlist && context->outer_itlist->has_non_vars) + { + newvar = search_indexed_tlist_for_non_var((Expr *) node, + context->outer_itlist, + OUTER_VAR); + if (newvar) + return (Node *) newvar; + } + if (context->inner_itlist && context->inner_itlist->has_non_vars) + { + newvar = search_indexed_tlist_for_non_var((Expr *) node, + context->inner_itlist, + INNER_VAR); + if (newvar) + return (Node *) newvar; + } + // XXX Is it an error to be here? + } + /* If not, do fix_param_node() */ return fix_param_node(context->root, (Param *) node); + } if (IsA(node, AlternativeSubPlan)) return fix_join_expr_mutator(fix_alternative_subplan(context->root, (AlternativeSubPlan *) node, @@ -3388,7 +3423,28 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context) } /* Special cases (apply only AFTER failing to match to lower tlist) */ if (IsA(node, Param)) + { + Param *param = (Param *) node; + /* + * If the Param is a PARAM_EXEC Param generated by an FDW, it should + * have bubbled up from a lower plan node; convert it into a simple + * Var referencing the output of the subplan. + */ + if (IS_FOREIGN_PARAM(context->root, param)) + { + if (context->subplan_itlist->has_non_vars) + { + newvar = search_indexed_tlist_for_non_var((Expr *) node, + context->subplan_itlist, + context->newvarno); + if (newvar) + return (Node *) newvar; + } + // XXX Is it an error to be here? + } + /* If not, do fix_param_node() */ return fix_param_node(context->root, (Param *) node); + } if (IsA(node, Aggref)) { Aggref *aggref = (Aggref *) node; diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 6aa8971c95d..67422914aa4 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -3194,7 +3194,12 @@ finalize_primnode(Node *node, finalize_primnode_context *context) { int paramid = ((Param *) node)->paramid; - context->paramids = bms_add_member(context->paramids, paramid); + /* + * Params added by FDWs are irrelevant for parameter change + * signaling. + */ + if (!bms_is_member(paramid, context->root->glob->foreignParamIDs)) + context->paramids = bms_add_member(context->paramids, paramid); } return false; /* no more to do here */ } diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c index 778e4662f6e..7089a267125 100644 --- a/src/backend/optimizer/util/appendinfo.c +++ b/src/backend/optimizer/util/appendinfo.c @@ -948,6 +948,29 @@ add_row_identity_var(PlannerInfo *root, Var *orig_var, root->processed_tlist = lappend(root->processed_tlist, tle); } +static void +fix_foreign_params(PlannerInfo *root, List *tlist) +{ + ListCell *lc; + + foreach(lc, tlist) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc); + Param *param = (Param *) tle->expr; + + if (tle->resjunk && IsA(param, Param) && + param->paramkind == PARAM_EXEC && + param->paramid == -1) + { + param->paramid = list_length(root->glob->paramExecTypes); + root->glob->paramExecTypes = + lappend_oid(root->glob->paramExecTypes, param->paramtype); + root->glob->foreignParamIDs = + bms_add_member(root->glob->foreignParamIDs, param->paramid); + } + } +} + /* * add_row_identity_columns * @@ -992,8 +1015,12 @@ add_row_identity_columns(PlannerInfo *root, Index rtindex, fdwroutine = GetFdwRoutineForRelation(target_relation, false); if (fdwroutine->AddForeignUpdateTargets != NULL) + { + fdwroutine->AddForeignUpdateTargets(root, rtindex, target_rte, target_relation); + fix_foreign_params(root, root->processed_tlist); + } /* * For UPDATE, we need to make the FDW fetch unchanged columns by diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c index 6a7b9edff3f..d8f08485e39 100644 --- a/src/backend/optimizer/util/inherit.c +++ b/src/backend/optimizer/util/inherit.c @@ -294,6 +294,7 @@ expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel, InvalidOid, 0); snprintf(resname, sizeof(resname), "tableoid%u", oldrc->rowmarkId); + tle = makeTargetEntry((Expr *) var, list_length(root->processed_tlist) + 1, pstrdup(resname), diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 687e923c46c..51294420987 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1320,6 +1320,27 @@ build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel, } continue; } + /* + * We allow FDWs to have PARAM_EXEC Params here. + */ + else if (IsA(var, Param)) + { + Param *param = (Param *) var; + + Assert(IS_FOREIGN_PARAM(root, param)); + + joinrel->reltarget->exprs = + lappend(joinrel->reltarget->exprs, param); + + /* + * Estimate using the type info (Note: keep this in sync with + * set_rel_width()) + */ + joinrel->reltarget->width += + get_typavgwidth(param->paramtype, param->paramtypmod); + + continue; + } /* * Otherwise, anything in a baserel or joinrel targetlist ought to be diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index 752ea9222f6..e382810b19f 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -329,18 +329,37 @@ apply_tlist_labeling(List *dest_tlist, List *src_tlist) ListCell *ld, *ls; - Assert(list_length(dest_tlist) == list_length(src_tlist)); - forboth(ld, dest_tlist, ls, src_tlist) + foreach(ld, dest_tlist) { + bool has_src = false; TargetEntry *dest_tle = (TargetEntry *) lfirst(ld); - TargetEntry *src_tle = (TargetEntry *) lfirst(ls); - - Assert(dest_tle->resno == src_tle->resno); - dest_tle->resname = src_tle->resname; - dest_tle->ressortgroupref = src_tle->ressortgroupref; - dest_tle->resorigtbl = src_tle->resorigtbl; - dest_tle->resorigcol = src_tle->resorigcol; - dest_tle->resjunk = src_tle->resjunk; + + foreach(ls, src_tlist) + { + TargetEntry *src_tle = (TargetEntry *) lfirst(ls); + + if (dest_tle->resno == src_tle->resno) + { + has_src = true; + dest_tle->resname = src_tle->resname; + dest_tle->ressortgroupref = src_tle->ressortgroupref; + dest_tle->resorigtbl = src_tle->resorigtbl; + dest_tle->resorigcol = src_tle->resorigcol; + dest_tle->resjunk = src_tle->resjunk; + break; + } + } + + if (!has_src) + { + TargetEntry *new_entry = palloc0(sizeof(TargetEntry)); + new_entry->resname = dest_tle->resname; + new_entry->ressortgroupref = dest_tle->ressortgroupref; + new_entry->resorigtbl = dest_tle->resorigtbl; + new_entry->resorigcol = dest_tle->resorigcol; + new_entry->resjunk = dest_tle->resjunk; + src_tlist = lappend(src_tlist, new_entry); + } } } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index b3aef797e7d..302bf7d2ff5 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9376,7 +9376,7 @@ get_parameter(Param *param, deparse_context *context) * It's a bug if we get here for anything except PARAM_EXTERN Params, but * in production builds printing $N seems more useful than failing. */ - Assert(param->paramkind == PARAM_EXTERN); + Assert(param->paramkind == PARAM_EXTERN || param->paramkind == PARAM_EXEC); appendStringInfo(context->buf, "$%d", param->paramid); } diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h index 77a6c48fd71..1f59f363a68 100644 --- a/src/include/access/htup_details.h +++ b/src/include/access/htup_details.h @@ -794,10 +794,17 @@ HeapTupleClearHeapOnly(const HeapTupleData *tuple) /* prototypes for functions in common/heaptuple.c */ extern Size heap_compute_data_size(TupleDesc tupleDesc, const Datum *values, const bool *isnull); +extern Size heap_compute_data_size_ext(TupleDesc tupleDesc, + const Datum *values, const bool *isnull, + int natts); extern void heap_fill_tuple(TupleDesc tupleDesc, const Datum *values, const bool *isnull, char *data, Size data_size, uint16 *infomask, uint8 *bit); +extern void heap_fill_tuple_ext(TupleDesc tupleDesc, + const Datum *values, const bool *isnull, + char *data, Size data_size, + uint16 *infomask, uint8 *bit, int natts); extern bool heap_attisnull(HeapTuple tup, int attnum, TupleDesc tupleDesc); extern Datum nocachegetattr(HeapTuple tup, int attnum, TupleDesc tupleDesc); @@ -806,10 +813,20 @@ extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, extern Datum getmissingattr(TupleDesc tupleDesc, int attnum, bool *isnull); extern HeapTuple heap_copytuple(HeapTuple tuple); + +struct TupleTableSlot; +extern HeapTuple heap_copytuple_ext(HeapTuple tuple, + struct TupleTableSlot *slot, + int dstnatts); + extern void heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest); extern Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc); extern HeapTuple heap_form_tuple(TupleDesc tupleDescriptor, const Datum *values, const bool *isnull); +extern HeapTuple heap_form_tuple_ext(TupleDesc tupleDescriptor, + const Datum *values, + const bool *isnull, + int natts); extern HeapTuple heap_modify_tuple(HeapTuple tuple, TupleDesc tupleDesc, const Datum *replValues, @@ -827,6 +844,11 @@ extern void heap_freetuple(HeapTuple htup); extern MinimalTuple heap_form_minimal_tuple(TupleDesc tupleDescriptor, const Datum *values, const bool *isnull, Size extra); +extern MinimalTuple heap_form_minimal_tuple_ext(TupleDesc tupleDescriptor, + const Datum *values, + const bool *isnull, + Size extra, + int natts); extern void heap_free_minimal_tuple(MinimalTuple mtup); extern MinimalTuple heap_copy_minimal_tuple(MinimalTuple mtup, Size extra); extern HeapTuple heap_tuple_from_minimal_tuple(MinimalTuple mtup); diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h index 3db6c9c9bd0..02ddc4fb79d 100644 --- a/src/include/executor/tuptable.h +++ b/src/include/executor/tuptable.h @@ -141,6 +141,7 @@ typedef struct TupleTableSlot MemoryContext tts_mcxt; /* slot itself is in this context */ ItemPointerData tts_tid; /* stored tuple's tid */ Oid tts_tableOid; /* table oid of tuple */ + Oid tts_remoteOid; } TupleTableSlot; /* routines for a TupleTableSlot implementation */ @@ -223,8 +224,10 @@ struct TupleTableSlotOps * meaningful "system columns" in the copy. The copy is not be "owned" by * the slot i.e. the caller has to take responsibility to free memory * consumed by the slot. + * + * TODO comment for "natts" */ - HeapTuple (*copy_heap_tuple) (TupleTableSlot *slot); + HeapTuple (*copy_heap_tuple) (TupleTableSlot *slot, int natts); /* * Return a copy of minimal tuple representing the contents of the slot. @@ -237,8 +240,10 @@ struct TupleTableSlotOps * The copy has "extra" bytes (maxaligned and zeroed) available before the * tuple, which is useful so that some callers may store extra data along * with the minimal tuple without the need for an additional allocation. + * + * TODO comment for "natts" */ - MinimalTuple (*copy_minimal_tuple) (TupleTableSlot *slot, Size extra); + MinimalTuple (*copy_minimal_tuple) (TupleTableSlot *slot, Size extra, int natts); }; /* @@ -368,6 +373,12 @@ extern void slot_getsomeattrs_int(TupleTableSlot *slot, int attnum); #ifndef FRONTEND +/* Forward declarations */ +static inline HeapTuple ExecCopySlotHeapTupleExteded(TupleTableSlot *slot, + int natts); +static inline MinimalTuple ExecCopySlotMinimalTupleExteded(TupleTableSlot *slot, + int natts); + /* * This function forces the entries of the slot's Datum/isnull arrays to be * valid at least up through the attnum'th entry. @@ -502,10 +513,16 @@ ExecMaterializeSlot(TupleTableSlot *slot) */ static inline HeapTuple ExecCopySlotHeapTuple(TupleTableSlot *slot) +{ + return ExecCopySlotHeapTupleExteded(slot, -1 /* natts */); +} + +static inline HeapTuple +ExecCopySlotHeapTupleExteded(TupleTableSlot *slot, int natts) { Assert(!TTS_EMPTY(slot)); - return slot->tts_ops->copy_heap_tuple(slot); + return slot->tts_ops->copy_heap_tuple(slot, natts); } /* @@ -514,7 +531,13 @@ ExecCopySlotHeapTuple(TupleTableSlot *slot) static inline MinimalTuple ExecCopySlotMinimalTuple(TupleTableSlot *slot) { - return slot->tts_ops->copy_minimal_tuple(slot, 0); + return ExecCopySlotMinimalTupleExteded(slot, -1 /* natts */); +} + +static inline MinimalTuple +ExecCopySlotMinimalTupleExteded(TupleTableSlot *slot, int natts) +{ + return slot->tts_ops->copy_minimal_tuple(slot, 0 /* no extra */, natts); } /* @@ -524,9 +547,9 @@ ExecCopySlotMinimalTuple(TupleTableSlot *slot) * caller to make an additional allocation). */ static inline MinimalTuple -ExecCopySlotMinimalTupleExtra(TupleTableSlot *slot, Size extra) +ExecCopySlotMinimalTupleExtra(TupleTableSlot *slot, Size extra, int natts) { - return slot->tts_ops->copy_minimal_tuple(slot, extra); + return slot->tts_ops->copy_minimal_tuple(slot, extra, natts); } /* @@ -545,9 +568,12 @@ ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) { Assert(!TTS_EMPTY(srcslot)); Assert(srcslot != dstslot); - Assert(dstslot->tts_tupleDescriptor->natts == - srcslot->tts_tupleDescriptor->natts); + /* + * If source slot has less attributes then target - copy source and + * set target to nulls. Otherwise copy only leading attributes and set + * target natts to source counter. + */ dstslot->tts_ops->copyslot(dstslot, srcslot); return dstslot; diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 27a2c6815b7..abe4904cc24 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -271,12 +271,19 @@ typedef struct PlannerGlobal /* extension state */ void **extension_state pg_node_attr(read_write_ignore); int extension_state_allocated; + + /* PARAM_EXEC Params generated by FDWs */ + Bitmapset *foreignParamIDs; } PlannerGlobal; /* macro for fetching the Plan associated with a SubPlan node */ #define planner_subplan_get_plan(root, subplan) \ ((Plan *) list_nth((root)->glob->subplans, (subplan)->plan_id - 1)) +/* macro for checking if a Param is a PARAM_EXEC Param generated by an FDW */ +#define IS_FOREIGN_PARAM(root, param) \ + ((param)->paramkind == PARAM_EXEC && \ + bms_is_member((param)->paramid, (root)->glob->foreignParamIDs)) /*---------- * PlannerInfo diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index cacef7d4151..e0433cd1f5e 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -402,6 +402,7 @@ typedef struct Param Oid paramcollid; /* token location, or -1 if unknown */ ParseLoc location; + Index target_rte; } Param; /* -- 2.43.0