On Mon, May 10, 2021 at 1:31 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, Apr 29, 2021 at 2:23 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Please find attached the latest patch set v74*
> >
> > Differences from v73* are:
> >
> > * Rebased to HEAD @ 2 days ago.
> >
> > * v74 addresses most of the feedback comments from Vignesh posts [1][2][3].
> >
>
> Thanks for the updated patch.
> Few comments:
> 1) I felt skey[2] should be skey as we are just using one key here.
>
> + ScanKeyData skey[2];
> + SysScanDesc scan;
> + bool has_subrels;
> +
> + rel = table_open(SubscriptionRelRelationId, AccessShareLock);
> +
> + ScanKeyInit(&skey[nkeys++],
> + Anum_pg_subscription_rel_srsubid,
> + BTEqualStrategyNumber, F_OIDEQ,
> + ObjectIdGetDatum(subid));
> +
> + scan = systable_beginscan(rel, InvalidOid, false,
> + NULL, nkeys, skey);
> +
>
Fixed in v75.
> 2) I felt we can change lsn data type from Int64 to XLogRecPtr
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> + The LSN of the prepare.
> +</para></listitem>
> +</varlistentry>
> +
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> + The end LSN of the transaction.
> +</para></listitem>
> +</varlistentry>
Deferred.
>
> 3) I felt we can change lsn data type from Int32 to TransactionId
> +<varlistentry>
> +<term>Int32</term>
> +<listitem><para>
> + Xid of the subtransaction (will be same as xid of the
> transaction for top-level
> + transactions).
> +</para></listitem>
> +</varlistentry>
Deferred.
>
> 4) Should we change this to "The end LSN of the prepared transaction"
> just to avoid any confusion of it meaning commit/rollback.
> +<varlistentry>
> +<term>Int64</term>
> +<listitem><para>
> + The end LSN of the transaction.
> +</para></listitem>
> +</varlistentry>
>
Modified in v75 for message types 'b', 'P', 'K', 'r', 'p'.
> Similar problems related to comments 2 and 3 are being discussed at
> [1], we can change it accordingly based on the conclusion in the other
> thread.
> [1] -
https://www.postgresql.org/message-id/flat/CAHut%2BPs2JsSd_OpBR9kXt1Rt4bwyXAjh875gUpFw6T210ttO7Q%40mail.gmail.com#cf2a85d0623dcadfbb1204a196681313
Yes, I will defer addressing those feedback comments 2 and 3 pending
the outcome of your other patch of the above thread.
----------
Kind Regards,
Peter Smith.
Fujitsu Australia