diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index cdd788f825..b108aad897 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -7730,6 +7730,45 @@ update utrtest set a = 1 where a = 1 or a = 2 returning *; 1 | qux triggered ! (1 row) +-- Check case where the foreign partition is a subplan target rel and +-- foreign parttion is locally modified (target table being joined +-- locally prevents a direct/remote modification plan). +explain (verbose, costs off) +update utrtest set a = 1 from (values (1), (2)) s(x) where a = s.x returning *; + QUERY PLAN +------------------------------------------------------------------------------ + Update on public.utrtest + Output: remp.a, remp.b, "*VALUES*".column1 + Foreign Update on public.remp + Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 RETURNING a, b + Update on public.locp + -> Hash Join + Output: 1, remp.b, remp.ctid, "*VALUES*".*, "*VALUES*".column1 + Hash Cond: (remp.a = "*VALUES*".column1) + -> Foreign Scan on public.remp + Output: remp.b, remp.ctid, remp.a + Remote SQL: SELECT a, b, ctid FROM public.loct FOR UPDATE + -> Hash + Output: "*VALUES*".*, "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Output: "*VALUES*".*, "*VALUES*".column1 + -> Hash Join + Output: 1, locp.b, locp.ctid, "*VALUES*".*, "*VALUES*".column1 + Hash Cond: (locp.a = "*VALUES*".column1) + -> Seq Scan on public.locp + Output: locp.b, locp.ctid, locp.a + -> Hash + Output: "*VALUES*".*, "*VALUES*".column1 + -> Values Scan on "*VALUES*" + Output: "*VALUES*".*, "*VALUES*".column1 +(24 rows) + +update utrtest set a = 1 from (values (1), (2)) s(x) where a = s.x returning *; + a | b | x +---+-----------------+--- + 1 | qux triggered ! | 1 +(1 row) + delete from utrtest; insert into utrtest values (2, 'qux'); -- Check case where the foreign partition isn't a subplan target rel diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 813286bba5..a27783cede 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1932,6 +1932,13 @@ update utrtest set a = 1 where a = 1 or a = 2 returning *; -- The new values are concatenated with ' triggered !' update utrtest set a = 1 where a = 1 or a = 2 returning *; +-- Check case where the foreign partition is a subplan target rel and +-- foreign parttion is locally modified (target table being joined +-- locally prevents a direct/remote modification plan). +explain (verbose, costs off) +update utrtest set a = 1 from (values (1), (2)) s(x) where a = s.x returning *; +update utrtest set a = 1 from (values (1), (2)) s(x) where a = s.x returning *; + delete from utrtest; insert into utrtest values (2, 'qux'); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 011e3cff1a..090f86c340 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -136,6 +136,11 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) { update_rri = mtstate->resultRelInfo; num_update_rri = list_length(node->plans); + /* + * Each of these will be set to either the leaf partition index + * to which a given subplan resultrel is mapped to or -1 if it + * cannot be used for tuple routing. + */ proute->subplan_partition_offsets = palloc(num_update_rri * sizeof(int)); proute->num_subplan_partition_offsets = num_update_rri; @@ -174,18 +179,29 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) if (update_rri_index < num_update_rri && RelationGetRelid(update_rri[update_rri_index].ri_RelationDesc) == leaf_oid) { - leaf_part_rri = &update_rri[update_rri_index]; - /* - * This is required in order to convert the partition's tuple to - * be compatible with the root partitioned table's tuple - * descriptor. When generating the per-subplan result rels, this - * was not set. + * Skip if an FDW is already using it. We must use a different + * one for tuple routing, because the latter will need its own + * FdwState. */ - leaf_part_rri->ri_PartitionRoot = rel; + if (update_rri[update_rri_index].ri_FdwState == NULL) + { + leaf_part_rri = &update_rri[update_rri_index]; - /* Remember the subplan offset for this ResultRelInfo */ - proute->subplan_partition_offsets[update_rri_index] = i; + /* + * This is required in order to convert the partition's tuple + * to be compatible with the root partitioned table's tuple + * descriptor. When generating the per-subplan result rels, + * this was not set. + */ + leaf_part_rri->ri_PartitionRoot = rel; + + /* Remember the subplan offset for this ResultRelInfo */ + proute->subplan_partition_offsets[update_rri_index] = i; + } + else + /* Mark this subplan's resultrel as unused. */ + proute->subplan_partition_offsets[update_rri_index] = -1; update_rri_index++; } @@ -195,8 +211,9 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) } /* - * For UPDATE, we should have found all the per-subplan resultrels in the - * leaf partitions. (If this is an INSERT, both values will be zero.) + * For UPDATE, we should have gone through all the per-subplan resultrels + * while matching against leaf partitions. (If this is an INSERT, both + * values will be zero.) */ Assert(update_rri_index == num_update_rri); @@ -892,21 +909,26 @@ ExecCleanupTupleRouting(ModifyTableState *mtstate, resultRelInfo); /* - * If this result rel is one of the UPDATE subplan result rels, let - * ExecEndPlan() close it. For INSERT or COPY, - * proute->subplan_partition_offsets will always be NULL. Note that - * the subplan_partition_offsets array and the partitions array have - * the partitions in the same order. So, while we iterate over - * partitions array, we also iterate over the - * subplan_partition_offsets array in order to figure out which of the - * result rels are present in the UPDATE subplans. + * If this partition routing result rel is one of the UPDATE subplan + * result rels, don't close it here, ExecEndPlan() will close it. + * For INSERT or COPY, proute->subplan_partition_offsets will always + * be NULL, because there are no subplan result rels in that case. + * Note that since partitions appear in the same order in both the + * subplan_partition_offsets array and the the partitions array, it's + * okay to match them like this. */ if (proute->subplan_partition_offsets && - subplan_index < proute->num_subplan_partition_offsets && - proute->subplan_partition_offsets[subplan_index] == i) + subplan_index < proute->num_subplan_partition_offsets) { - subplan_index++; - continue; + /* skip subplan result rels that were not reused. */ + while (proute->subplan_partition_offsets[subplan_index] < 0) + subplan_index++; + + if (proute->subplan_partition_offsets[subplan_index] == i) + { + subplan_index++; + continue; + } } ExecCloseIndices(resultRelInfo);