Re: Add contrib/pg_logicalsnapinspect

On Fri, Aug 30, 2024 at 5:18 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> On Fri, Aug 30, 2024 at 03:43:12PM +0530, Amit Kapila wrote:
> > On Thu, Aug 29, 2024 at 6:33 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Aug 29, 2024 at 3:44 PM Bertrand Drouvot
> > > <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> > > >
> > > > Yeah that's fair. And now I'm wondering if we need an extra module. I think
> > > > we could "simply" expose 2 new functions in core, thoughts?
> > > >
> > > > > > What do you think? Did you have something else in mind?
> > > > > >
> > > > >
> > > > > On similar lines, we can also provide a function to get the slot's
> > > > > on-disk data.
> > > >
> > > > Yeah, having a way to expose the data from the disk makes fully sense to me.
> > > >
> > > > > IIRC, Bharath had previously proposed a tool to achieve
> > > > > the same. It is fine if we don't want to add that as part of this
> > > > > patch but I mentioned it because by having that we can have a set of
> > > > > functions to view logical decoding data.
> > > >
> > > > That's right. I think this one would be simply enough to expose one or two
> > > > functions in core too (and probably would not need an extra module).
> > >
> > > +1 for functions in core unless this extra module
> > > pg_logicalsnapinspect works as a tool to be helpful even when the
> > > server is down.
> > >
> >
> > We have an example of pageinspect which provides low-level functions
> > to aid debugging. The proposal for these APIs also seems to fall in
> > the same category,
>
> That's right, but...
>
> > so why go for the core for these functions?
>
> as we decided not to expose the SnapBuildOnDisk and SnapBuild structs to public
> and to create/expose 2 new functions in snapbuild.c then the functions in the
> module would do nothing but expose the data coming from the snapbuild.c's
> functions (get the tuple and send it to the client). That sounds weird to me to
> create a module that would "only" do so, that's why I thought that in core
> functions taking care of everything make more sense.
>

I see your point. Does anyone else have an opinion on the need for
these functions and whether to expose them from a contrib module or
have them as core functions?

--
With Regards,
Amit Kapila.