Horton

Re: Add contrib/pg_logicalsnapinspect

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-09-09T10:54:09Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix indentation issue

  2. pg_logicalinspect: Stabilize isolation tests.

  3. Add contrib/pg_logicalinspect.

  4. Move SnapBuild and SnapBuildOnDisk structs to snapshot_internal.h.

On Fri, Aug 30, 2024 at 5:18 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.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.rupireddyforpostgres@gmail.com> wrote:
> > >
> > > On Thu, Aug 29, 2024 at 3:44 PM Bertrand Drouvot
> > > <bertranddrouvot.pg@gmail.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.