public inbox for [email protected]
help / color / mirror / Atom feedFrom: Masahiko Sawada <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: [email protected]
Subject: Re: Add contrib/pg_logicalsnapinspect
Date: Wed, 9 Oct 2024 10:21:31 -0700
Message-ID: <CAD21AoCAEXmxaDHAjMwqcpV6JVa91wBbXamQbk-rv1QnYhzGhA@mail.gmail.com> (raw)
In-Reply-To: <ZwY68OhKx/[email protected]>
References: <[email protected]>
<CAHut+PsVj9+DO6_zQrW8-n47DxuxFzwHKXOJi6AE3M9Yki9XPA@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAD21AoBHkL-C22pgGJCA_8Jv6HjYq0c_1Y_b2Xktk7_SdUqxqA@mail.gmail.com>
<ZwOkPDCKQ/[email protected]>
<CAHut+PvtLb6g1L9C_zbufuHbJTWNq-BeseCpVgjsabG0md6_8g@mail.gmail.com>
<[email protected]>
<CAD21AoAO3zyooPQ+E2rpEi1YBc+gxZr0CUtJwxaFJ-WWimJNvA@mail.gmail.com>
<ZwY68OhKx/[email protected]>
On Wed, Oct 9, 2024 at 1:12 AM Bertrand Drouvot
<[email protected]> wrote:
>
> Hi,
>
> On Tue, Oct 08, 2024 at 10:52:11AM -0700, Masahiko Sawada wrote:
> > On Tue, Oct 8, 2024 at 9:25 AM Bertrand Drouvot
> > <[email protected]> wrote:
> >
> > Thank you for updating the patch! I have some comments on v12 patch:
>
> Thanks for looking at it!
>
> > ---
> > + if (ondisk.builder.committed.xcnt > 0)
> > + {
> > + Datum *arrayelems;
> > + int narrayelems = 0;
> > +
> > + arrayelems = (Datum *)
> > palloc(ondisk.builder.committed.xcnt * sizeof(Datum));
> > +
> > + for (; narrayelems < ondisk.builder.committed.xcnt;
> > narrayelems++)
> > + arrayelems[narrayelems] =
> > Int64GetDatum((int64) ondisk.builder.committed.xip[narrayelems]);
> > +
> > + values[i++] =
> > PointerGetDatum(construct_array_builtin(arrayelems, narrayelems,
> > INT8OID));
> > + }
> >
> > Since committed_xip and catchange_xip are xid[], we should use
> > TransactionIdGetDatum() and XIDOID instead.
>
> I ended up using INT8OID because XIDOID is not part of the switch in
> construct_array_builtin() and so leads to:
>
> "
> ERROR: type 28 not supported by construct_array_builtin()
> "
Thank you for pointing it out.
>
> One option could be (did not test it) to add this switch in construct_array_builtin():
>
> + case XIDOID:
> + elmlen = sizeof(TransactionId);
> + elmbyval = true;
> + elmalign = TYPALIGN_INT;
> + break;
>
> I think that could make sense and would probably need a dedicated patch for that,
> thoughts?
Or can we use construct_array() instead?
> > ---
> > +# Test the pg_logicalinspect functions: that needs some permutation to
> > +# ensure that we are creating multiple logical snapshots and that one of them
> > +# contains ongoing catalogs changes.
> >
> > If we use prepared transactions modifying catalog changes, can we
> > write the normal (i.e. not isolation check) tests? It would be easier
> > to write and add tests.
> >
>
> Not sure about this one. I think that the test is simple enough and mainly inspired
> by what can be found in the test_decoding module.
>
> We could still add "normal" (REGRESS) tests in the future should we add features
> to the pg_logicalinspect module that would require new tests.
>
> For example, test_decoding is using both kind of tests, what do you think?
Fair point. I agree with you.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
view thread (36+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add contrib/pg_logicalsnapinspect
In-Reply-To: <CAD21AoCAEXmxaDHAjMwqcpV6JVa91wBbXamQbk-rv1QnYhzGhA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox