public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3 2/2] New command ALTER TABLE ALL IN TABLESPACE SET LOGGED/UNLOGGED 4+ messages / 3 participants [nested] [flat]
* [PATCH v3 2/2] New command ALTER TABLE ALL IN TABLESPACE SET LOGGED/UNLOGGED @ 2020-11-11 14:21 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Kyotaro Horiguchi @ 2020-11-11 14:21 UTC (permalink / raw) To ease invoking ALTER TABLE SET LOGGED/UNLOGGED, this command changes relation persistence of all tables in the specified tablespace. --- src/backend/commands/tablecmds.c | 140 +++++++++++++++++++++++++++++++ src/backend/nodes/copyfuncs.c | 16 ++++ src/backend/nodes/equalfuncs.c | 15 ++++ src/backend/parser/gram.y | 20 +++++ src/backend/tcop/utility.c | 11 +++ src/include/commands/tablecmds.h | 2 + src/include/nodes/nodes.h | 1 + src/include/nodes/parsenodes.h | 9 ++ 8 files changed, 214 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 37a15d31ee..2f65abb19b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13696,6 +13696,146 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt) return new_tablespaceoid; } +/* + * Alter Table ALL ... SET LOGGED/UNLOGGED + * + * Allows a user to change persistence of all objects in a given tablespace in + * the current database. Objects can be chosen based on the owner of the + * object also, to allow users to change persistene only their objects. The + * main permissions handling is done by the lower-level change persistence + * function. + * + * All to-be-modified objects are locked first. If NOWAIT is specified and the + * lock can't be acquired then we ereport(ERROR). + */ +void +AlterTableSetLoggedAll(AlterTableSetLoggedAllStmt *stmt) +{ + List *relations = NIL; + ListCell *l; + ScanKeyData key[1]; + Relation rel; + TableScanDesc scan; + HeapTuple tuple; + Oid tablespaceoid; + List *role_oids = roleSpecsToIds(NIL); + + /* Ensure we were not asked to change something we can't */ + if (stmt->objtype != OBJECT_TABLE) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("only tables can be specified"))); + + /* Get the tablespace OID */ + tablespaceoid = get_tablespace_oid(stmt->tablespacename, false); + + /* + * Now that the checks are done, check if we should set either to + * InvalidOid because it is our database's default tablespace. + */ + if (tablespaceoid == MyDatabaseTableSpace) + tablespaceoid = InvalidOid; + + /* + * Walk the list of objects in the tablespace to pick up them. This will + * only find objects in our database, of course. + */ + ScanKeyInit(&key[0], + Anum_pg_class_reltablespace, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(tablespaceoid)); + + rel = table_open(RelationRelationId, AccessShareLock); + scan = table_beginscan_catalog(rel, 1, key); + while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + { + Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple); + Oid relOid = relForm->oid; + + /* + * Do not pick-up objects in pg_catalog as part of this, if an admin + * really wishes to do so, they can issue the individual ALTER + * commands directly. + * + * Also, explicitly avoid any shared tables, temp tables, or TOAST + * (TOAST will be changed with the main table). + */ + if (IsCatalogNamespace(relForm->relnamespace) || + relForm->relisshared || + isAnyTempNamespace(relForm->relnamespace) || + IsToastNamespace(relForm->relnamespace)) + continue; + + /* Only pick up the object type requested */ + if (relForm->relkind != RELKIND_RELATION) + continue; + + /* Check if we are only picking-up objects owned by certain roles */ + if (role_oids != NIL && !list_member_oid(role_oids, relForm->relowner)) + continue; + + /* + * Handle permissions-checking here since we are locking the tables + * and also to avoid doing a bunch of work only to fail part-way. Note + * that permissions will also be checked by AlterTableInternal(). + * + * Caller must be considered an owner on the table of which we're going + * to change persistence. + */ + if (!pg_class_ownercheck(relOid, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relOid)), + NameStr(relForm->relname)); + + if (stmt->nowait && + !ConditionalLockRelationOid(relOid, AccessExclusiveLock)) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_IN_USE), + errmsg("aborting because lock on relation \"%s.%s\" is not available", + get_namespace_name(relForm->relnamespace), + NameStr(relForm->relname)))); + else + LockRelationOid(relOid, AccessExclusiveLock); + + /* + * Add to our list of objects of which we're going to change + * persistence. + */ + relations = lappend_oid(relations, relOid); + } + + table_endscan(scan); + table_close(rel, AccessShareLock); + + if (relations == NIL) + ereport(NOTICE, + (errcode(ERRCODE_NO_DATA_FOUND), + errmsg("no matching relations in tablespace \"%s\" found", + tablespaceoid == InvalidOid ? "(database default)" : + get_tablespace_name(tablespaceoid)))); + + /* + * Everything is locked, loop through and change persistence of all of the + * relations. + */ + foreach(l, relations) + { + List *cmds = NIL; + AlterTableCmd *cmd = makeNode(AlterTableCmd); + + if (stmt->logged) + cmd->subtype = AT_SetLogged; + else + cmd->subtype = AT_SetUnLogged; + + cmds = lappend(cmds, cmd); + + EventTriggerAlterTableStart((Node *) stmt); + /* OID is set by AlterTableInternal */ + AlterTableInternal(lfirst_oid(l), cmds, false); + EventTriggerAlterTableEnd(); + } +} + static void index_copy_data(Relation rel, RelFileNode newrnode) { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index ba3ccc712c..127da5151d 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4138,6 +4138,19 @@ _copyAlterTableMoveAllStmt(const AlterTableMoveAllStmt *from) return newnode; } +static AlterTableSetLoggedAllStmt * +_copyAlterTableSetLoggedAllStmt(const AlterTableSetLoggedAllStmt *from) +{ + AlterTableSetLoggedAllStmt *newnode = makeNode(AlterTableSetLoggedAllStmt); + + COPY_STRING_FIELD(tablespacename); + COPY_SCALAR_FIELD(objtype); + COPY_SCALAR_FIELD(logged); + COPY_SCALAR_FIELD(nowait); + + return newnode; +} + static CreateExtensionStmt * _copyCreateExtensionStmt(const CreateExtensionStmt *from) { @@ -5441,6 +5454,9 @@ copyObjectImpl(const void *from) case T_AlterTableMoveAllStmt: retval = _copyAlterTableMoveAllStmt(from); break; + case T_AlterTableSetLoggedAllStmt: + retval = _copyAlterTableSetLoggedAllStmt(from); + break; case T_CreateExtensionStmt: retval = _copyCreateExtensionStmt(from); break; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index a2ef853dc2..4f13a1762b 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1872,6 +1872,18 @@ _equalAlterTableMoveAllStmt(const AlterTableMoveAllStmt *a, return true; } +static bool +_equalAlterTableSetLoggedAllStmt(const AlterTableSetLoggedAllStmt *a, + const AlterTableSetLoggedAllStmt *b) +{ + COMPARE_STRING_FIELD(tablespacename); + COMPARE_SCALAR_FIELD(objtype); + COMPARE_SCALAR_FIELD(logged); + COMPARE_SCALAR_FIELD(nowait); + + return true; +} + static bool _equalCreateExtensionStmt(const CreateExtensionStmt *a, const CreateExtensionStmt *b) { @@ -3494,6 +3506,9 @@ equal(const void *a, const void *b) case T_AlterTableMoveAllStmt: retval = _equalAlterTableMoveAllStmt(a, b); break; + case T_AlterTableSetLoggedAllStmt: + retval = _equalAlterTableSetLoggedAllStmt(a, b); + break; case T_CreateExtensionStmt: retval = _equalCreateExtensionStmt(a, b); break; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 31c95443a5..2222fd8fe3 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -1934,6 +1934,26 @@ AlterTableStmt: n->nowait = $13; $$ = (Node *)n; } + | ALTER TABLE ALL IN_P TABLESPACE name SET LOGGED opt_nowait + { + AlterTableSetLoggedAllStmt *n = + makeNode(AlterTableSetLoggedAllStmt); + n->tablespacename = $6; + n->objtype = OBJECT_TABLE; + n->logged = true; + n->nowait = $9; + $$ = (Node *)n; + } + | ALTER TABLE ALL IN_P TABLESPACE name SET UNLOGGED opt_nowait + { + AlterTableSetLoggedAllStmt *n = + makeNode(AlterTableSetLoggedAllStmt); + n->tablespacename = $6; + n->objtype = OBJECT_TABLE; + n->logged = false; + n->nowait = $9; + $$ = (Node *)n; + } | ALTER INDEX qualified_name alter_table_cmds { AlterTableStmt *n = makeNode(AlterTableStmt); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 53a511f1da..16606448bf 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -161,6 +161,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree) case T_AlterTSConfigurationStmt: case T_AlterTSDictionaryStmt: case T_AlterTableMoveAllStmt: + case T_AlterTableSetLoggedAllStmt: case T_AlterTableSpaceOptionsStmt: case T_AlterTableStmt: case T_AlterTypeStmt: @@ -1732,6 +1733,12 @@ ProcessUtilitySlow(ParseState *pstate, commandCollected = true; break; + case T_AlterTableSetLoggedAllStmt: + AlterTableSetLoggedAll((AlterTableSetLoggedAllStmt *) parsetree); + /* commands are stashed in AlterTableSetLoggedAll */ + commandCollected = true; + break; + case T_DropStmt: ExecDropStmt((DropStmt *) parsetree, isTopLevel); /* no commands stashed for DROP */ @@ -2619,6 +2626,10 @@ CreateCommandTag(Node *parsetree) tag = AlterObjectTypeCommandTag(((AlterTableMoveAllStmt *) parsetree)->objtype); break; + case T_AlterTableSetLoggedAllStmt: + tag = AlterObjectTypeCommandTag(((AlterTableSetLoggedAllStmt *) parsetree)->objtype); + break; + case T_AlterTableStmt: tag = AlterObjectTypeCommandTag(((AlterTableStmt *) parsetree)->objtype); break; diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 08c463d3c4..646928466d 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -42,6 +42,8 @@ extern void AlterTableInternal(Oid relid, List *cmds, bool recurse); extern Oid AlterTableMoveAll(AlterTableMoveAllStmt *stmt); +extern void AlterTableSetLoggedAll(AlterTableSetLoggedAllStmt *stmt); + extern ObjectAddress AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema); diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index caed683ba9..16d91d3e1d 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -424,6 +424,7 @@ typedef enum NodeTag T_AlterCollationStmt, T_CallStmt, T_AlterStatsStmt, + T_AlterTableSetLoggedAllStmt, /* * TAGS FOR PARSE TREE NODES (parsenodes.h) diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index dc2bb40926..c3eab6f1ab 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2253,6 +2253,15 @@ typedef struct AlterTableMoveAllStmt bool nowait; } AlterTableMoveAllStmt; +typedef struct AlterTableSetLoggedAllStmt +{ + NodeTag type; + char *tablespacename; + ObjectType objtype; /* Object type to move */ + bool logged; + bool nowait; +} AlterTableSetLoggedAllStmt; + /* ---------------------- * Create/Alter Extension Statements * ---------------------- -- 2.27.0 ----Next_Part(Fri_Jan__8_14_47_05_2021_579)---- ^ permalink raw reply [nested|flat] 4+ messages in thread
* Allow FDW extensions to support MERGE command via CustomScan @ 2024-12-12 09:28 Önder Kalacı <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Önder Kalacı @ 2024-12-12 09:28 UTC (permalink / raw) To: pgsql-hackers Hi hackers, Currently, it is not possible for any fdw extension to support Merge command, as that's prohibited in the parser. In this proposal, we allow extensions to support Merge command via `CustomScan` node by moving the Merge support check from parser to planner. For existing fdw, they don't have to change anything. I changed postgres_fdw mostly for documentation purposes. See the attached patch. Thanks, Onder KALACI Attachments: [application/octet-stream] 0001-Allow-FDW-extensions-to-support-MERGE-command-via-Cu.patch (7.4K, ../../CACawEhXGVpBqt=UK0grzZK_Bsdy6q2-EoGR6x5yAUz5UwpCjiA@mail.gmail.com/3-0001-Allow-FDW-extensions-to-support-MERGE-command-via-Cu.patch) download | inline diff: From 80d4e853fe6197228612e4312d690f66c1f818c4 Mon Sep 17 00:00:00 2001 From: Onder KALACI <[email protected]> Date: Fri, 6 Dec 2024 11:07:26 +0300 Subject: [PATCH] Allow FDW extensions to support MERGE command via CustomScan Currently, it is not possible for any fdw extension to support Merge command, as that's prohibited in the parser. In this commit, we allow extensions to support Merge command via `CustomScan` node by moving the Merge support check from parser to planner. For existing fdw, they don't have to change anyting. We change postgres_fdw mostly for documentation purposes. --- contrib/postgres_fdw/postgres_fdw.c | 16 ++++++++++++++++ doc/src/sgml/fdwhandler.sgml | 25 +++++++++++++++++++++++++ src/backend/optimizer/plan/createplan.c | 16 ++++++++++------ src/backend/parser/parse_merge.c | 12 ++++++++++-- src/include/foreign/fdwapi.h | 5 +++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 67 insertions(+), 8 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c0810fbd7c..541a57575c 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -376,6 +376,7 @@ static void postgresBeginForeignInsert(ModifyTableState *mtstate, static void postgresEndForeignInsert(EState *estate, ResultRelInfo *resultRelInfo); static int postgresIsForeignRelUpdatable(Relation rel); +static bool postgresIsForeignServerMergeCapable(void); static bool postgresPlanDirectModify(PlannerInfo *root, ModifyTable *plan, Index resultRelation, @@ -574,6 +575,7 @@ postgres_fdw_handler(PG_FUNCTION_ARGS) routine->BeginForeignInsert = postgresBeginForeignInsert; routine->EndForeignInsert = postgresEndForeignInsert; routine->IsForeignRelUpdatable = postgresIsForeignRelUpdatable; + routine->IsForeignServerMergeCapable = postgresIsForeignServerMergeCapable; routine->PlanDirectModify = postgresPlanDirectModify; routine->BeginDirectModify = postgresBeginDirectModify; routine->IterateDirectModify = postgresIterateDirectModify; @@ -2348,6 +2350,20 @@ postgresIsForeignRelUpdatable(Relation rel) (1 << CMD_INSERT) | (1 << CMD_UPDATE) | (1 << CMD_DELETE) : 0; } + +/* + * postgresIsForeignServerMergeCapable + * Determine whether the foreign server is capable of merge. Core code (ExecMerge()) + * doesn't support merge on foreign tables, so we always return false. Some FDWs + * may support merge via CustomScan nodes, in which case they should return true. + */ +static bool +postgresIsForeignServerMergeCapable(void) +{ + /* postgres_fdw does not support CMD_MERGE */ + return false; +} + /* * postgresRecheckForeignScan * Execute a local join execution plan for a foreign join diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index b80320504d..818ae93e7e 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -1288,6 +1288,31 @@ RecheckForeignScan(ForeignScanState *node, </para> </sect2> + <sect2 id="fdw-callbacks-merge"> + <title>FDW Routines for <command>MERGE</command></title> + + <para> +<programlisting> +bool +IsForeignServerMergeCapable(); +</programlisting> + + Postgres doesn't support <command>MERGE</command> on foreign tables, + see <function>ExecMerge</function>. Still, extensions may provide + custom scan nodes to support <command>MERGE</command> on foreign + tables. If your extension provides such custom scan node, this + function should return true. + </para> + + <para> + If the <function>IsForeignServerMergeCapable</function> pointer is set to + <literal>NULL</literal> or returns <literal>false</literal>, + <command>MERGE</command> fails on the planning phase with a proper + error message. + </para> + + </sect2> + <sect2 id="fdw-callbacks-explain"> <title>FDW Routines for <command>EXPLAIN</command></title> diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 178c572b02..de6418ac0b 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -7242,13 +7242,17 @@ make_modifytable(PlannerInfo *root, Plan *subplan, */ if (operation == CMD_MERGE && fdwroutine != NULL) { - RangeTblEntry *rte = planner_rt_fetch(rti, root); + /* Check if the foreign table is mergeable */ + if (!fdwroutine->IsForeignServerMergeCapable || !fdwroutine->IsForeignServerMergeCapable()) + { + RangeTblEntry *rte = planner_rt_fetch(rti, root); - ereport(ERROR, - errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot execute MERGE on relation \"%s\"", - get_rel_name(rte->relid)), - errdetail_relkind_not_supported(rte->relkind)); + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot execute MERGE on relation \"%s\"", + get_rel_name(rte->relid)), + errdetail_relkind_not_supported(rte->relkind)); + } } /* diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c index 87df79027d..97db9f11f4 100644 --- a/src/backend/parser/parse_merge.c +++ b/src/backend/parser/parse_merge.c @@ -194,10 +194,18 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt) false, targetPerms); qry->mergeTargetRelation = qry->resultRelation; - /* The target relation must be a table or a view */ + /* + * The target relation must be a table or a view. + * + * Although the Merge command on foreign tables are allowed in the + * grammar, it is not natively supported for foreign tables. We allow the + * parser so that we give extensions a chance to support it via custom + * scan nodes. + */ if (pstate->p_target_relation->rd_rel->relkind != RELKIND_RELATION && pstate->p_target_relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && - pstate->p_target_relation->rd_rel->relkind != RELKIND_VIEW) + pstate->p_target_relation->rd_rel->relkind != RELKIND_VIEW && + pstate->p_target_relation->rd_rel->relkind != RELKIND_FOREIGN_TABLE) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot execute MERGE on relation \"%s\"", diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h index fcde3876b2..13a4fc0e25 100644 --- a/src/include/foreign/fdwapi.h +++ b/src/include/foreign/fdwapi.h @@ -115,6 +115,8 @@ typedef void (*EndForeignInsert_function) (EState *estate, typedef int (*IsForeignRelUpdatable_function) (Relation rel); +typedef bool (*IsForeignServerMergeCapable_function) (void); + typedef bool (*PlanDirectModify_function) (PlannerInfo *root, ModifyTable *plan, Index resultRelation, @@ -248,6 +250,9 @@ typedef struct FdwRoutine RefetchForeignRow_function RefetchForeignRow; RecheckForeignScan_function RecheckForeignScan; + /* Support functions for MERGE */ + IsForeignServerMergeCapable_function IsForeignServerMergeCapable; + /* Support functions for EXPLAIN */ ExplainForeignScan_function ExplainForeignScan; ExplainForeignModify_function ExplainForeignModify; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index ce33e55bf1..3c38e9f98c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1268,6 +1268,7 @@ IpcSemaphoreId IpcSemaphoreKey IsForeignPathAsyncCapable_function IsForeignRelUpdatable_function +IsForeignServerMergeCapable_function IsForeignScanParallelSafe_function IsoConnInfo IspellDict -- 2.43.0 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Allow FDW extensions to support MERGE command via CustomScan @ 2024-12-12 14:20 Matheus Alcantara <[email protected]> parent: Önder Kalacı <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Matheus Alcantara @ 2024-12-12 14:20 UTC (permalink / raw) To: Önder Kalacı <[email protected]>; +Cc: pgsql-hackers Hi, Just some thoughts on documentation part. > +</programlisting> > + > + Postgres doesn't support <command>MERGE</command> on foreign tables, > + see <function>ExecMerge</function>. Still, extensions may provide > + custom scan nodes to support <command>MERGE</command> on foreign > + tables. If your extension provides such custom scan node, this > + function should return true. > + </para> What's the point about mentioning the ExecMerge function? I didn't find any relevant documentation about why it not support foreign tables, maybe its because it should not? I didn't understand what is a "custom scan node" on the fdw context at first place (I don't know if it is an already know word on this context), but from what I've understood so far, to a fdw extension support MERGE it should implements on PlanForeignModify right? If that's the case maybe it's worth updating the PlanForeignModify documentation as well? It only mention insert, update, or delete operations. Also, I don't know if would be good to link the PlanForeignModify on this part of the documentation, WYT? -- Matheus Alcantara EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Allow FDW extensions to support MERGE command via CustomScan @ 2024-12-13 06:35 Önder Kalacı <[email protected]> parent: Matheus Alcantara <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Önder Kalacı @ 2024-12-13 06:35 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: pgsql-hackers HI Matheus, all > > > +</programlisting> > > + > > + Postgres doesn't support <command>MERGE</command> on foreign tables, > > + see <function>ExecMerge</function>. Still, extensions may provide > > + custom scan nodes to support <command>MERGE</command> on foreign > > + tables. If your extension provides such custom scan node, this > > + function should return true. > > + </para> > > What's the point about mentioning the ExecMerge function? I didn't > find any relevant documentation about why it not support foreign > tables, maybe its because it should not? Exec{Insert|Update|Delete} functions, they all have the common pattern of code flow, where they call the fdw registered functions via ExecForeign{Insert|Update|Delete}. And, that's where each fdw implementation decides what to do on {Insert|Update|Delete} such as postgres_fdw does in postgresExecForeignInsert. At this time, ExecMerge() ignores fdws as Merge command is prohibited for foreign tables in the parser. So, it is guaranteed that ExecMerge() won't run on a foreign table. Overall, it would probably be an improvement to mention on ExecMerge() function comment that foreign tables are not supported. > > I didn't understand what is a "custom scan node" on the fdw context at > first place (I don't know if it is an already know word on this > context), but from what I've understood so far, to a fdw extension > support MERGE it should implements on PlanForeignModify right? In the long term, I think that's a good plan. First, the core code in ExecMerge() should be aware of foreign tables, then each foreign table should handle Merge command planning on its own PlanForeignModify. That'd be great, because the execution of Merge command is pretty complex, and in essence Postgres would be providing the solid infrastructure for all foreign tables. However, I expect that to be a non-trivial patch. Instead, the goal of this patch is to at least let extensions to completely override the planning & execution via CustomScan, not confined to Postgres' foreign table planning & execution. Then, it is completely the responsibility of the extension developer to handle the Merge command. Today, that's not possible, and CustomScan's are the recommended** way to implement. To recap the goal of this patch: Do not change anything for the existing fdws, but at least give the willing fdw extensions to have a way to implement Merge command via CustomScan. Thanks, Onder ** https://www.postgresql.org/docs/current/custom-scan.html ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-12-13 06:35 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-11-11 14:21 [PATCH v3 2/2] New command ALTER TABLE ALL IN TABLESPACE SET LOGGED/UNLOGGED Kyotaro Horiguchi <[email protected]> 2024-12-12 09:28 Allow FDW extensions to support MERGE command via CustomScan Önder Kalacı <[email protected]> 2024-12-12 14:20 ` Re: Allow FDW extensions to support MERGE command via CustomScan Matheus Alcantara <[email protected]> 2024-12-13 06:35 ` Re: Allow FDW extensions to support MERGE command via CustomScan Önder Kalacı <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox