Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Shlok Kyal <shlok.kyal.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Hou, Zhijie/侯 志杰 <houzj.fnst@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2024-07-24T06:40:01Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Doc: Add documentation for sequence synchronization.

  2. Remove unused assignment in CREATE PUBLICATION grammar.

  3. Add seq_sync_error_count to subscription statistics.

  4. Fix few issues in commit 5509055d69.

  5. Add sequence synchronization for logical replication.

  6. Add worker type argument to logical replication worker functions.

  7. Introduce "REFRESH SEQUENCES" for subscriptions.

  8. Refactor logical worker synchronization code into a separate file.

  9. Standardize use of REFRESH PUBLICATION in code and messages.

  10. Add "ALL SEQUENCES" support to publications.

  11. Expose sequence page LSN via pg_get_sequence_data.

  12. Resume conflict-relevant data retention automatically.

  13. Post-commit review fixes for 228c370868.

  14. Generate GUC tables from .dat file

On Tue, 23 Jul 2024 at 11:03, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here are some review comments for patch v20240720-0002.
>
> ======
> 1. Commit message:
>
> 1a.
> The commit message is stale. It is still referring to functions and
> views that have been moved to patch 0003.

Modified

> 1b.
> "ALL SEQUENCES" is not a command. It is a clause of the CREATE
> PUBLICATION command.

Modified

> src/bin/psql/describe.c
>
> 2. describeOneTableDetails
>
> Although it's not the fault of this patch, this patch propagates the
> confusion of 'result' versus 'res'. Basically, I did not understand
> the need for the variable 'result'. There is already a "PGResult
> *res", and unless I am mistaken we can just keep re-using that instead
> of introducing a 2nd variable having almost the same name and purpose.

This is intentional, we cannot clear res as it will be used in many
places of printTable like in
printTable->print_aligned_text->pg_wcssize which was earlier stored
from printTableAddCell calls.

The rest of the nitpicks comments were merged.
The v20240724 version patch attached at [1] has the changes for the same.

[1] - https://www.postgresql.org/message-id/CALDaNm1uncevCSMqo5Nk%3DtqqV_o3KNH_jwp8URiGop_nPC8BTg%40mail.gmail.com

Regards,
Vignesh