| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Nikhil Sontakke <nikhil(at)planetscale(dot)com> |
| Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Subject: | Re: DDL deparse |
| Date: | 2026-11-12 00:17:20 |
| Message-ID: | CAD21AoC8iYWs6Q4P887vH0kmaN=4tNML+xx4DmtQLLR_y5Pc8w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
Sorry for the late reply.
On Wed, Oct 14, 2026 at 12:48 AM Nikhil Sontakke <nikhil(at)planetscale(dot)com> wrote:
>
>>
>> I've rebased and updated the patch.
>>
>
> I reviewed and tested the v2 series. And found some issues that I attempted to fix via
> two follow-up patches, to apply in order on top of your four-patch series.
>
> The first addresses three correctness cases:
>
> * Repeated identity clauses on the same column currently match the first
> raw subcommand more than once. The patch matches successive collected
> identity subcommands to successive raw occurrences.
>
> * ADD COLUMN b int DEFAULT 1 followed by ALTER COLUMN b SET DEFAULT 2
> currently reads the final catalog default for both clauses. The patch
> captures each relevant subcommand's cooked default when it is collected,
> preserving the distinct effects on existing rows and future inserts.
>
> * Table-level IF EXISTS is preserved for ordinary ALTER TABLE, column and
> constraint renames, and SET SCHEMA. Table rename already handles it.
>
> The second patch adds SQL regression coverage for nested DDL. I'm happy to
> report that these cases worked with the existing deparsing logic. The harness changes
> use the existing settings without adding another GUC.
Thank you for the report and the patch!
I've reviewed and fixed them. I'll post the updated patch soon.
>
> I know that full deparse support is still pending :-) (and am happy to contribute towards
> full deparse support as well) but looking ahead to DDLR, do you have a message
> format in mind for conveying the deparsed commands to subscribers?
>
> The deparser already produces a JSON envelope that can be expanded into
> SQL. Would you carry that JSON in a transactional logical message, or
> expand it on the publisher and send SQL with a separate metadata envelope?
I think we should send the JSON and let the subscriber expand it to
SQL. What I have in mind is close to what the previous DDL replication
patch did: a dedicated WAL record, always transactional, decoded into
a new protocol message. The message would have:
- xid
- flags
- LSN of the DDL record
- relid
- the JSON
The JSON envelope gets a format version and the role that executed the
command. For example, "ALTER TABLE public.t ADD COLUMN b int" executed
by alice:
{
"version": 1,
"tag": "ALTER TABLE",
"role": "alice",
"command": {
"fmt": "ALTER TABLE%{only}s %{identity}D %{subcmds:, }s",
"only": null,
"identity": {"schemaname": "public", "objname": "t"},
"subcmds": [
{
"fmt": "ADD COLUMN%{if_not_exists}s %{definition}s",
"if_not_exists": null,
"definition": {
"fmt": "%{name}I
%{coltype}T%{storage}s%{compression}s%{collation}s%{not_null}s%{default}s%{identity_column}s%{generated_column}s",
"name": "b",
"coltype": {"schemaname": "pg_catalog", "typename":
"int4", "typmod": "", "typarray": false},
"storage": null, "compression": null, "collation": null,
"not_null": null,
"default": null, "identity_column": null, "generated_column": null
}
}
]
}
}
>
> This also seems closely related to your subscriber logical-message hook
> work. Do you see that hook as the intended entry point for DDL apply, or
> would you prefer a dedicated logical replication protocol message?
>
I think DDL replication should have its own WAL record and its own
protocol message, rather than going through the logical-message hook.
As I wrote in the hook thread[1], pg_logical_emit_message() is granted
to PUBLIC, so if the apply worker executed DDL carried by a generic
logical message, any user who can connect to the publisher could
create
or alter tables on the subscriber with the apply worker's privileges.
Also, an extension handling such messages can't easily respect the
publication's table list or publish_via_partition_root. The hook is
meant for applications that use generic messages on the subscriber,
and the in-core DDL apply would not go through it.
Regards,
[1] https://postgr.es/m/CAD21AoATy+CzYmNYp_fKi4zsGLEGOmZen3OR+hxsCqn2GsQBoQ@mail.gmail.com
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-11-12 00:27:02 | Re: Postgres fdw: Bug in InitPgFdwOptions initialization can cause an esoteric crash |
| Previous Message | Zsolt Parragi | 2026-11-12 00:05:35 | Re: Interrupts vs signals |