From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Peter Smith' <smithpb2250(at)gmail(dot)com> |
Cc: | Julien Rouhaud <rjuju123(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | RE: [PoC] pg_upgrade: allow to upgrade publisher node |
Date: | 2023-04-12 07:55:28 |
Message-ID: | TYAPR01MB58667700921DAB8A3AFEBAF6F59B9@TYAPR01MB5866.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Peter,
Thank you for giving comments. PSA new version.
> src/bin/pg_dump/pg_backup.h
>
> 1.
> + int logical_slot_only;
>
> The field should be plural - "logical_slots_only"
Fixed.
> src/bin/pg_dump/pg_dump.c
>
> 2.
> + appendPQExpBufferStr(query,
> + "SELECT r.slot_name, r.plugin, r.two_phase "
> + "FROM pg_replication_slots r "
> + "WHERE r.database = current_database() AND temporary = false "
> + "AND wal_status IN ('reserved', 'extended');");
>
> The alias 'r' may not be needed at all here, but since you already
> have it IMO it looks a bit strange that you used it for only some of
> the columns but not others.
Right, I removed alias. Moreover, the namespace 'pg_catalog' is now specified.
> 3.
> +
> + /* FIXME: force dumping */
> + slotinfo[i].dobj.dump = DUMP_COMPONENT_ALL;
>
> Why the "FIXME" here? Are you intending to replace this code with
> something else?
I was added FIXME because I was not sure whether we must add selectDumpable...()
function was needed or not. Now I have been thinking that such a functions are not
needed, so replaced comments. More detail, please see following:
Replication slots cannot be a member of extension because pg_create_logical_replication_slot()
cannot be called within the install script. This means that checkExtensionMembership()
is not needed. Moreover, we do not have have any options to include/exclude slots
in dumping, so checking their name like selectDumpableExtension() is not needed.
Based on them, I think the function is not needed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachment | Content-Type | Size |
---|---|---|
v5-0001-pg_upgrade-Add-include-logical-replication-slots-.patch | application/octet-stream | 24.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2023-04-12 08:54:54 | Re: When to drop src/tools/msvc support |
Previous Message | Thomas Munro | 2023-04-12 07:37:42 | Re: Direct I/O |