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
