On Thu, Apr 29, 2021 at 2:23 PM Peter Smith <smithpb2...@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); + 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> 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> 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> 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 Regards, Vignesh
