From b605edd3687835908eedd770c44cfd41d85b97ba Mon Sep 17 00:00:00 2001 From: Arseny Sher Date: Tue, 31 Oct 2017 14:14:18 +0300 Subject: [PATCH 1/4] Adapted for https://github.com/arssher/postgresql/tree/foreign_copy_from. --- src/utility_stmt_hooking.c | 52 ++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index 103f194e..93908d38 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -499,7 +499,7 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, /* Initialize ResultPartsStorage */ init_result_parts_storage(&parts_storage, estate, false, ResultPartsStorageStandard, - prepare_rri_for_copy, NULL); + prepare_rri_for_copy, cstate); parts_storage.saved_rel_info = parent_result_rel; /* Set up a tuple slot too */ @@ -634,13 +634,20 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, /* Check the constraints of the tuple */ if (child_result_rel->ri_RelationDesc->rd_att->constr) ExecConstraints(child_result_rel, slot, estate); + if (!child_result_rel->ri_FdwRoutine) + { + /* OK, store the tuple and create index entries for it */ + simple_heap_insert(child_result_rel->ri_RelationDesc, tuple); - /* OK, store the tuple and create index entries for it */ - simple_heap_insert(child_result_rel->ri_RelationDesc, tuple); - - if (child_result_rel->ri_NumIndices > 0) - recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self), - estate, false, NULL, NIL); + if (child_result_rel->ri_NumIndices > 0) + recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self), + estate, false, NULL, NIL); + } + else /* FDW table */ + { + child_result_rel->ri_FdwRoutine->ForeignNextCopyFrom( + estate, child_result_rel, cstate); + } /* AFTER ROW INSERT Triggers (FIXME: NULL transition) */ ExecARInsertTriggersCompat(estate, child_result_rel, tuple, @@ -677,6 +684,24 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, ExecResetTupleTable(estate->es_tupleTable, false); + { + /* Shut down FDWs. TODO: make hook in fini_result_parts_storage? */ + HASH_SEQ_STATUS stat; + ResultRelInfoHolder *rri_holder; /* ResultRelInfo holder */ + + hash_seq_init(&stat, parts_storage.result_rels_table); + while ((rri_holder = (ResultRelInfoHolder *) hash_seq_search(&stat)) != NULL) + { + ResultRelInfo *resultRelInfo = rri_holder->result_rel_info; + + if (resultRelInfo->ri_FdwRoutine) + { + resultRelInfo->ri_FdwRoutine->EndForeignCopyFrom( + estate, resultRelInfo); + } + } + } + /* Close partitions and destroy hash table */ fini_result_parts_storage(&parts_storage, true); @@ -689,7 +714,7 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, } /* - * COPY FROM does not support FDWs, emit ERROR. + * Init COPY FROM, if supported. */ static void prepare_rri_for_copy(EState *estate, @@ -699,10 +724,17 @@ prepare_rri_for_copy(EState *estate, { ResultRelInfo *rri = rri_holder->result_rel_info; FdwRoutine *fdw_routine = rri->ri_FdwRoutine; + CopyState cstate = (CopyState) arg; if (fdw_routine != NULL) - elog(ERROR, "cannot copy to foreign partition \"%s\"", - get_rel_name(RelationGetRelid(rri->ri_RelationDesc))); + { + if (!FdwCopyFromIsSupported(fdw_routine)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("FDW adapter for relation \"%s\" doesn't support COPY FROM", + RelationGetRelationName(rri->ri_RelationDesc)))); + rri->ri_FdwRoutine->BeginForeignCopyFrom(estate, rri, cstate); + } } /* From 74f40cc87b4a547b6261e7b0c80277477f59bb13 Mon Sep 17 00:00:00 2001 From: Arseny Sher Date: Thu, 2 Nov 2017 18:49:35 +0300 Subject: [PATCH 2/4] COPYing FROM to parent table instead of foreign, assuming we using shardman. Also, callback args simplified. --- src/include/partition_filter.h | 16 +++---- src/partition_filter.c | 82 +++++++++++++++----------------- src/utility_stmt_hooking.c | 85 ++++++++++++++++------------------ 3 files changed, 84 insertions(+), 99 deletions(-) diff --git a/src/include/partition_filter.h b/src/include/partition_filter.h index 85ddcf91..0cd08c36 100644 --- a/src/include/partition_filter.h +++ b/src/include/partition_filter.h @@ -43,17 +43,15 @@ typedef struct } ResultRelInfoHolder; -/* Forward declaration (for on_new_rri_holder()) */ +/* Forward declaration (for on_rri_holder()) */ struct ResultPartsStorage; typedef struct ResultPartsStorage ResultPartsStorage; /* - * Callback to be fired at rri_holder creation. + * Callback to be fired at rri_holder creation/destruction. */ -typedef void (*on_new_rri_holder)(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg); +typedef void (*on_rri_holder)(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); /* * Cached ResultRelInfos of partitions. @@ -66,7 +64,7 @@ struct ResultPartsStorage bool speculative_inserts; /* for ExecOpenIndices() */ - on_new_rri_holder on_new_rri_holder_callback; + on_rri_holder on_new_rri_holder_callback; void *callback_arg; EState *estate; /* pointer to executor's state */ @@ -116,11 +114,11 @@ void init_result_parts_storage(ResultPartsStorage *parts_storage, EState *estate, bool speculative_inserts, Size table_entry_size, - on_new_rri_holder on_new_rri_holder_cb, + on_rri_holder on_new_rri_holder_cb, void *on_new_rri_holder_cb_arg); void fini_result_parts_storage(ResultPartsStorage *parts_storage, - bool close_rels); + bool close_rels, on_rri_holder hook); ResultRelInfoHolder * scan_result_parts_storage(Oid partid, ResultPartsStorage *storage); diff --git a/src/partition_filter.c b/src/partition_filter.c index 214b926a..a1886c4d 100644 --- a/src/partition_filter.c +++ b/src/partition_filter.c @@ -68,18 +68,12 @@ CustomScanMethods partition_filter_plan_methods; CustomExecMethods partition_filter_exec_methods; -static void prepare_rri_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg); -static void prepare_rri_returning_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg); -static void prepare_rri_fdw_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg); +static void prepare_rri_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); +static void prepare_rri_returning_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); +static void prepare_rri_fdw_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); static Node *fix_returning_list_mutator(Node *node, void *state); static Index append_rte_to_estate(EState *estate, RangeTblEntry *rte); @@ -143,7 +137,7 @@ init_result_parts_storage(ResultPartsStorage *parts_storage, EState *estate, bool speculative_inserts, Size table_entry_size, - on_new_rri_holder on_new_rri_holder_cb, + on_rri_holder on_new_rri_holder_cb, void *on_new_rri_holder_cb_arg) { HASHCTL *result_rels_table_config = &parts_storage->result_rels_table_config; @@ -177,16 +171,21 @@ init_result_parts_storage(ResultPartsStorage *parts_storage, /* Free ResultPartsStorage (close relations etc) */ void -fini_result_parts_storage(ResultPartsStorage *parts_storage, bool close_rels) +fini_result_parts_storage(ResultPartsStorage *parts_storage, bool close_rels, + on_rri_holder hook) { HASH_SEQ_STATUS stat; ResultRelInfoHolder *rri_holder; /* ResultRelInfo holder */ - /* Close partitions and free free conversion-related stuff */ - if (close_rels) + hash_seq_init(&stat, parts_storage->result_rels_table); + while ((rri_holder = (ResultRelInfoHolder *) hash_seq_search(&stat)) != NULL) { - hash_seq_init(&stat, parts_storage->result_rels_table); - while ((rri_holder = (ResultRelInfoHolder *) hash_seq_search(&stat)) != NULL) + /* Call destruction hook, if needed */ + if (hook != NULL) + hook(rri_holder, parts_storage); + + /* Close partitions and free free conversion-related stuff */ + if (close_rels) { ExecCloseIndices(rri_holder->result_rel_info); @@ -202,13 +201,8 @@ fini_result_parts_storage(ResultPartsStorage *parts_storage, bool close_rels) free_conversion_map(rri_holder->tuple_map); } - } - - /* Else just free conversion-related stuff */ - else - { - hash_seq_init(&stat, parts_storage->result_rels_table); - while ((rri_holder = (ResultRelInfoHolder *) hash_seq_search(&stat)) != NULL) + /* Else just free conversion-related stuff */ + else { /* Skip if there's no map */ if (!rri_holder->tuple_map) @@ -329,10 +323,8 @@ scan_result_parts_storage(Oid partid, ResultPartsStorage *parts_storage) /* Call on_new_rri_holder_callback() if needed */ if (parts_storage->on_new_rri_holder_callback) - parts_storage->on_new_rri_holder_callback(parts_storage->estate, - rri_holder, - parts_storage, - parts_storage->callback_arg); + parts_storage->on_new_rri_holder_callback(rri_holder, + parts_storage); /* Finally append ResultRelInfo to storage->es_alloc_result_rels */ append_rri_to_estate(parts_storage->estate, child_result_rel_info); @@ -702,7 +694,7 @@ partition_filter_end(CustomScanState *node) PartitionFilterState *state = (PartitionFilterState *) node; /* Executor will close rels via estate->es_result_relations */ - fini_result_parts_storage(&state->result_parts, false); + fini_result_parts_storage(&state->result_parts, false, NULL); Assert(list_length(node->custom_ps) == 1); ExecEndNode((PlanState *) linitial(node->custom_ps)); @@ -793,21 +785,17 @@ pfilter_build_tlist(Relation parent_rel, List *tlist) /* Main trigger */ static void -prepare_rri_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg) +prepare_rri_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage) { - prepare_rri_returning_for_insert(estate, rri_holder, rps_storage, arg); - prepare_rri_fdw_for_insert(estate, rri_holder, rps_storage, arg); + prepare_rri_returning_for_insert(rri_holder, rps_storage); + prepare_rri_fdw_for_insert(rri_holder, rps_storage); } /* Prepare 'RETURNING *' tlist & projection */ static void -prepare_rri_returning_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg) +prepare_rri_returning_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage) { PartitionFilterState *pfstate; List *returning_list; @@ -815,12 +803,15 @@ prepare_rri_returning_for_insert(EState *estate, *parent_rri; Index parent_rt_idx; TupleTableSlot *result_slot; + EState *estate; + + estate = rps_storage->estate; /* We don't need to do anything ff there's no map */ if (!rri_holder->tuple_map) return; - pfstate = (PartitionFilterState *) arg; + pfstate = (PartitionFilterState *) rps_storage->callback_arg; returning_list = pfstate->returning_list; /* Exit if there's no RETURNING list */ @@ -857,14 +848,15 @@ prepare_rri_returning_for_insert(EState *estate, /* Prepare FDW access structs */ static void -prepare_rri_fdw_for_insert(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg) +prepare_rri_fdw_for_insert(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage) { ResultRelInfo *rri = rri_holder->result_rel_info; FdwRoutine *fdw_routine = rri->ri_FdwRoutine; Oid partid; + EState *estate; + + estate = rps_storage->estate; /* Nothing to do if not FDW */ if (fdw_routine == NULL) diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index 93908d38..6ad88bf6 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -64,10 +64,10 @@ static uint64 PathmanCopyFrom(CopyState cstate, List *range_table, bool old_protocol); -static void prepare_rri_for_copy(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg); +static void prepare_rri_for_copy(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); +static void finish_rri_copy(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage); /* @@ -105,20 +105,6 @@ is_pathman_related_copy(Node *parsetree) /* Check that relation is partitioned */ if (get_pathman_relation_info(parent_relid)) { - ListCell *lc; - - /* Analyze options list */ - foreach (lc, copy_stmt->options) - { - DefElem *defel = (DefElem *) lfirst(lc); - - Assert(IsA(defel, DefElem)); - - /* We do not support freeze */ - if (strcmp(defel->defname, "freeze") == 0) - elog(ERROR, "freeze is not supported for partitioned tables"); - } - /* Emit ERROR if we can't see the necessary symbols */ #ifdef DISABLE_PATHMAN_COPY elog(ERROR, "COPY is not supported for partitioned tables on Windows"); @@ -481,6 +467,10 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, uint64 processed = 0; + /* We do not support freeze */ + if (cstate->freeze) + elog(ERROR, "freeze is not supported for partitioned tables"); + tupDesc = RelationGetDescr(parent_rel); @@ -684,26 +674,8 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, ExecResetTupleTable(estate->es_tupleTable, false); - { - /* Shut down FDWs. TODO: make hook in fini_result_parts_storage? */ - HASH_SEQ_STATUS stat; - ResultRelInfoHolder *rri_holder; /* ResultRelInfo holder */ - - hash_seq_init(&stat, parts_storage.result_rels_table); - while ((rri_holder = (ResultRelInfoHolder *) hash_seq_search(&stat)) != NULL) - { - ResultRelInfo *resultRelInfo = rri_holder->result_rel_info; - - if (resultRelInfo->ri_FdwRoutine) - { - resultRelInfo->ri_FdwRoutine->EndForeignCopyFrom( - estate, resultRelInfo); - } - } - } - /* Close partitions and destroy hash table */ - fini_result_parts_storage(&parts_storage, true); + fini_result_parts_storage(&parts_storage, true, finish_rri_copy); /* Close parent's indices */ ExecCloseIndices(parent_result_rel); @@ -717,23 +689,46 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, * Init COPY FROM, if supported. */ static void -prepare_rri_for_copy(EState *estate, - ResultRelInfoHolder *rri_holder, - const ResultPartsStorage *rps_storage, - void *arg) +prepare_rri_for_copy(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage) { - ResultRelInfo *rri = rri_holder->result_rel_info; - FdwRoutine *fdw_routine = rri->ri_FdwRoutine; - CopyState cstate = (CopyState) arg; + ResultRelInfo *rri = rri_holder->result_rel_info; + FdwRoutine *fdw_routine = rri->ri_FdwRoutine; + CopyState cstate = (CopyState) rps_storage->callback_arg; + ResultRelInfo *parent_rri; + const char *parent_relname; + EState *estate; + + estate = rps_storage->estate; if (fdw_routine != NULL) { + parent_rri = rps_storage->saved_rel_info; + parent_relname = psprintf( + "%s.%s", "public", + quote_identifier(RelationGetRelationName(parent_rri->ri_RelationDesc))); if (!FdwCopyFromIsSupported(fdw_routine)) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("FDW adapter for relation \"%s\" doesn't support COPY FROM", RelationGetRelationName(rri->ri_RelationDesc)))); - rri->ri_FdwRoutine->BeginForeignCopyFrom(estate, rri, cstate); + fdw_routine->BeginForeignCopyFrom(estate, rri, cstate, parent_relname); + } +} + +/* + * Shut down FDWs. + */ +static void +finish_rri_copy(ResultRelInfoHolder *rri_holder, + const ResultPartsStorage *rps_storage) +{ + ResultRelInfo *resultRelInfo = rri_holder->result_rel_info; + + if (resultRelInfo->ri_FdwRoutine) + { + resultRelInfo->ri_FdwRoutine->EndForeignCopyFrom( + rps_storage->estate, resultRelInfo); } } From 551f4f23dfbfe67dd04b77b823c2ba74dda72b66 Mon Sep 17 00:00:00 2001 From: Arseny Sher Date: Fri, 1 Dec 2017 12:26:59 +0300 Subject: [PATCH 3/4] Do COPY FROM to foreign parts only when needed. That is, when 1) pg_pathman was compiled against postgres with shardman patches. 2) Shardman's COPY FROM was explicitly asked by setting renderzvous var. Also, check for 'freeze' option early, as before, to keep regression tests as they are. --- src/utility_stmt_hooking.c | 88 +++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index 6ad88bf6..e8ddd3de 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -22,6 +22,7 @@ #include "access/xact.h" #include "catalog/namespace.h" #include "commands/copy.h" +#include "commands/defrem.h" #include "commands/trigger.h" #include "commands/tablecmds.h" #include "foreign/fdwapi.h" @@ -105,6 +106,26 @@ is_pathman_related_copy(Node *parsetree) /* Check that relation is partitioned */ if (get_pathman_relation_info(parent_relid)) { + ListCell *lc; + + /* Analyze options list */ + foreach (lc, copy_stmt->options) + { + DefElem *defel = lfirst_node(DefElem, lc); + + /* We do not support freeze */ + /* + * It would be great to allow copy.c extract option value and + * check it ready. However, there is no possibility (hooks) to do + * that before messaging 'ok, begin streaming data' to the client, + * which is ugly and confusing: e.g. it would require us to + * actually send something in regression tests before we notice + * the error. + */ + if (strcmp(defel->defname, "freeze") == 0 && defGetBoolean(defel)) + elog(ERROR, "freeze is not supported for partitioned tables"); + } + /* Emit ERROR if we can't see the necessary symbols */ #ifdef DISABLE_PATHMAN_COPY elog(ERROR, "COPY is not supported for partitioned tables on Windows"); @@ -467,11 +488,6 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, uint64 processed = 0; - /* We do not support freeze */ - if (cstate->freeze) - elog(ERROR, "freeze is not supported for partitioned tables"); - - tupDesc = RelationGetDescr(parent_rel); parent_result_rel = makeNode(ResultRelInfo); @@ -633,11 +649,13 @@ PathmanCopyFrom(CopyState cstate, Relation parent_rel, recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false, NULL, NIL); } +#ifdef PG_SHARDMAN else /* FDW table */ { child_result_rel->ri_FdwRoutine->ForeignNextCopyFrom( estate, child_result_rel, cstate); } +#endif /* AFTER ROW INSERT Triggers (FIXME: NULL transition) */ ExecARInsertTriggersCompat(estate, child_result_rel, tuple, @@ -694,25 +712,51 @@ prepare_rri_for_copy(ResultRelInfoHolder *rri_holder, { ResultRelInfo *rri = rri_holder->result_rel_info; FdwRoutine *fdw_routine = rri->ri_FdwRoutine; - CopyState cstate = (CopyState) rps_storage->callback_arg; - ResultRelInfo *parent_rri; - const char *parent_relname; - EState *estate; - - estate = rps_storage->estate; if (fdw_routine != NULL) { - parent_rri = rps_storage->saved_rel_info; - parent_relname = psprintf( - "%s.%s", "public", - quote_identifier(RelationGetRelationName(parent_rri->ri_RelationDesc))); - if (!FdwCopyFromIsSupported(fdw_routine)) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("FDW adapter for relation \"%s\" doesn't support COPY FROM", - RelationGetRelationName(rri->ri_RelationDesc)))); - fdw_routine->BeginForeignCopyFrom(estate, rri, cstate, parent_relname); + /* + * If this Postgres has no idea about shardman, behave as usual: + * vanilla Postgres doesn't support COPY FROM to foreign partitions. + * However, shardman patches to core extend FDW API to allow it, + * though currently postgres_fdw does so in a bit perverted way: we + * redirect COPY FROM to parent table on foreign server, assuming it + * exists, and let it direct tuple to proper partition. This is + * because otherwise we have to modify logic of managing connections + * in postgres_fdw and keep many connections open to one server from + * one backend. + */ +#ifndef PG_SHARDMAN + goto bail_out; /* to avoid 'unused label' warning */ +#else + { /* separate block to avoid 'unused var' warnings */ + CopyState cstate = (CopyState) rps_storage->callback_arg; + ResultRelInfo *parent_rri; + const char *parent_relname; + EState *estate; + + /* shardman COPY FROM requested? */ + if (*find_rendezvous_variable( + "shardman_pathman_copy_from_rendezvous") == NULL) + goto bail_out; + + estate = rps_storage->estate; + parent_rri = rps_storage->saved_rel_info; + parent_relname = psprintf( + "%s.%s", "public", + quote_identifier(RelationGetRelationName(parent_rri->ri_RelationDesc))); + if (!FdwCopyFromIsSupported(fdw_routine)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("FDW adapter for relation \"%s\" doesn't support COPY FROM", + RelationGetRelationName(rri->ri_RelationDesc)))); + fdw_routine->BeginForeignCopyFrom(estate, rri, cstate, parent_relname); + return; + } +#endif +bail_out: + elog(ERROR, "cannot copy to foreign partition \"%s\"", + get_rel_name(RelationGetRelid(rri->ri_RelationDesc))); } } @@ -723,6 +767,7 @@ static void finish_rri_copy(ResultRelInfoHolder *rri_holder, const ResultPartsStorage *rps_storage) { +#ifdef PG_SHARDMAN ResultRelInfo *resultRelInfo = rri_holder->result_rel_info; if (resultRelInfo->ri_FdwRoutine) @@ -730,6 +775,7 @@ finish_rri_copy(ResultRelInfoHolder *rri_holder, resultRelInfo->ri_FdwRoutine->EndForeignCopyFrom( rps_storage->estate, resultRelInfo); } +#endif } /* From d7520bbf4d50e514a88b5925daf3989219ded480 Mon Sep 17 00:00:00 2001 From: Arseny Sher Date: Fri, 1 Dec 2017 16:21:36 +0300 Subject: [PATCH 4/4] Code simpified and improved a bit. --- src/utility_stmt_hooking.c | 44 +++++++++++--------------------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/src/utility_stmt_hooking.c b/src/utility_stmt_hooking.c index e8ddd3de..e64c1542 100644 --- a/src/utility_stmt_hooking.c +++ b/src/utility_stmt_hooking.c @@ -718,43 +718,23 @@ prepare_rri_for_copy(ResultRelInfoHolder *rri_holder, /* * If this Postgres has no idea about shardman, behave as usual: * vanilla Postgres doesn't support COPY FROM to foreign partitions. - * However, shardman patches to core extend FDW API to allow it, - * though currently postgres_fdw does so in a bit perverted way: we - * redirect COPY FROM to parent table on foreign server, assuming it - * exists, and let it direct tuple to proper partition. This is - * because otherwise we have to modify logic of managing connections - * in postgres_fdw and keep many connections open to one server from - * one backend. + * However, shardman patches to core extend FDW API to allow it. */ -#ifndef PG_SHARDMAN - goto bail_out; /* to avoid 'unused label' warning */ -#else - { /* separate block to avoid 'unused var' warnings */ +#ifdef PG_SHARDMAN + /* shardman COPY FROM requested? */ + if (*find_rendezvous_variable( + "shardman_pathman_copy_from_rendezvous") != NULL && + FdwCopyFromIsSupported(fdw_routine)) + { CopyState cstate = (CopyState) rps_storage->callback_arg; - ResultRelInfo *parent_rri; - const char *parent_relname; - EState *estate; - - /* shardman COPY FROM requested? */ - if (*find_rendezvous_variable( - "shardman_pathman_copy_from_rendezvous") == NULL) - goto bail_out; - - estate = rps_storage->estate; - parent_rri = rps_storage->saved_rel_info; - parent_relname = psprintf( - "%s.%s", "public", - quote_identifier(RelationGetRelationName(parent_rri->ri_RelationDesc))); - if (!FdwCopyFromIsSupported(fdw_routine)) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("FDW adapter for relation \"%s\" doesn't support COPY FROM", - RelationGetRelationName(rri->ri_RelationDesc)))); - fdw_routine->BeginForeignCopyFrom(estate, rri, cstate, parent_relname); + ResultRelInfo *parent_rri = rps_storage->saved_rel_info; + EState *estate = rps_storage->estate; + + fdw_routine->BeginForeignCopyFrom(estate, rri, cstate, parent_rri); return; } #endif -bail_out: + elog(ERROR, "cannot copy to foreign partition \"%s\"", get_rel_name(RelationGetRelid(rri->ri_RelationDesc))); }