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? -- With Regards, Amit Kapila.
