On Mon, May 10, 2021 at 1:31 PM vignesh C <vignes...@gmail.com> wrote: > > 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); > + >
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
