Re: Add contrib/pg_logicalsnapinspect

Hi,

On Wed, Oct 09, 2024 at 10:21:31AM -0700, Masahiko Sawada wrote:
> On Wed, Oct 9, 2024 at 1:12 AM Bertrand Drouvot
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> > 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?

I had a closer look to d746021de1 (which introduced construct_array_builtin())
and the hackers thread that lead to it [1].

IIUC, the idea was to:

1. centralize the hardcoded knowledge that were in the calls to construct_array()
and deconstruct_array() for built-in types
2. notational simplification and bug-proofing

As XIDOID is a built-in type, I think that it would make sense to add it in
deconstruct_array_builtin()/construct_array_builtin().

I think the reason XIDOID has not been added in d746021de1 is that there were no
use case at that time (means no existing calls to construct_array()/deconstruct_array()
with hardcoded XIDOID related arguments).

One could say that we would just add 2 calls to construct_array() in the pg_logicalinspect
module but, for example, d746021de1 also took care of CSTRINGOID that had a single
call at that time:

$ git show d746021de1 | grep deconstruct_array_builtin | grep -c CSTRINGOID
1
$ git show d746021de1 | grep construct_array_builtin | grep -v deconstruct_array_builtin | grep -c CSTRINGOID
1

So I think that having construct_array_builtin()/deconstruct_array_builtin()
taking care of XIDOID is the way to go. If that makes sense to you then I'll
submit a dedicated patch for it, thoughts?

[1]: https://www.postgresql.org/message-id/flat/2914356f-9e5f-8c59-2995-5997fc48bcba%40enterprisedb.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com