Subject: Allow FDW extensions to support MERGE command via CustomScan

From: Önder Kalacı <[email-masked]>
Date: Thu, 12 Dec 2024 12:28:22 +0300

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

---

From: Matheus Alcantara <[email-masked]>
Date: Thu, 12 Dec 2024 11:20:19 -0300

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

---

From: Önder Kalacı <[email-masked]>
Date: Fri, 13 Dec 2024 09:35:45 +0300

HI Matheus, all

> 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.

> 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