Horton

Re: Add contrib/pg_logicalsnapinspect

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, shveta malik <shveta.malik@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-10-11T01:59:33Z
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.

Hi, Here are a few comments for patch set v13*

//////////

Patch v13-0001

======
Commit message

1.1
/were no use case/was no use case/

~~~

1.2
It seemed a bit odd that the switch cases for
'construct_array_builtin' are not the same as those for
'deconstruct_array_builtin'.

For example, all these ones seem missing from deconstruct:
    case INT4OID:
    case INT8OID:
    case NAMEOID:
    case REGTYPEOID:

I know that has nothing to do with your patch, and I guess it does not
cause any problems otherwise there would be ERRORs. But, if you are to
follow this same current pattern, then perhaps you don't need to add
your new case for 'deconstruct_array_builtin', since AFAICT you are
never using it.

//////////

Patch v13-0002

======
pg_get_logical_snapshot_meta:

2.1
+ Datum values[PG_GET_LOGICAL_SNAPSHOT_META_COLS];
+ bool nulls[PG_GET_LOGICAL_SNAPSHOT_META_COLS];

FWIW, if you wanted to avoid a few lines you could initialise the
nulls array during the declaration.
bool nulls[PG_GET_LOGICAL_SNAPSHOT_META_COLS] = {0};

This seems a common pattern in other source code, and it replaces the
need for the subsequent memset.

~~~

pg_get_logical_snapshot_info:

2.2
+ Datum values[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS];
+ bool nulls[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS];

Ditto of #2.1. You could instead just initialise in the declaration like:
bool nulls[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS] = {0};

======
Kind Regards,
Peter Smith.
Fujitsu Australia