On Sat, Feb 27, 2021 at 8:29 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Feb 26, 2021 at 7:26 PM vignesh C <vignes...@gmail.com> wrote: > > > > On Fri, Feb 26, 2021 at 4:13 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > > > On Fri, Feb 26, 2021 at 7:47 PM Ajin Cherian <itsa...@gmail.com> wrote: > > > > > > > I've updated snapshot_was_exported_at_ member to pg_replication_slots > > > > as well. > > > > Do have a look and let me know if there are any comments. > > > > > > Update with both patches. > > > > Thanks for fixing and providing an updated patch. Patch applies, make > > check and make check-world passes. I could see the issue working fine. > > > > Few minor comments: > > + <structfield>snapshot_was_exported_at</structfield> > > <type>pg_lsn</type> > > + </para> > > + <para> > > + The address (<literal>LSN</literal>) at which the logical > > + slot found a consistent point at the time of slot creation. > > + <literal>NULL</literal> for physical slots. > > + </para></entry> > > + </row> > > > > > > I had seen earlier also we had some discussion on naming > > snapshot_was_exported_at. Can we change snapshot_was_exported_at to > > snapshot_exported_lsn, I felt if we can include the lsn in the name, > > the user will be able to interpret easily and also it will be similar > > to other columns in pg_replication_slots view. > > > > I have recommended above to change this name to initial_consistency_at > because there are times when we don't export snapshot and we still set > this like when creating slots with CRS_NOEXPORT_SNAPSHOT or when > creating via SQL APIs. I am not sure why Ajin neither changed the > name nor responded to that comment. What is your opinion?
initial_consistency_at looks good to me. That is more understandable. Regards, Vignesh
