Horton

Re: Add contrib/pg_logicalsnapinspect

shveta malik <shveta.malik@gmail.com>

View thread
From: shveta malik <shveta.malik@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers@lists.postgresql.org, shveta malik <shveta.malik@gmail.com>
Date: 2024-09-24T04:05:14Z
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 Mon, Sep 23, 2024 at 7:57 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> My review comments for v8-0001
>
> ======
> contrib/pg_logicalinspect/pg_logicalinspect.c
>
> 1.
> +/*
> + * Lookup table for SnapBuildState.
> + */
> +
> +#define SNAPBUILD_STATE_INCR 1
> +
> +static const char *const SnapBuildStateDesc[] = {
> + [SNAPBUILD_START + SNAPBUILD_STATE_INCR] = "start",
> + [SNAPBUILD_BUILDING_SNAPSHOT + SNAPBUILD_STATE_INCR] = "building",
> + [SNAPBUILD_FULL_SNAPSHOT + SNAPBUILD_STATE_INCR] = "full",
> + [SNAPBUILD_CONSISTENT + SNAPBUILD_STATE_INCR] = "consistent",
> +};
> +
> +/*
>
> nit - the SNAPBUILD_STATE_INCR made this code appear more complicated
> than it is.

I agree.

> Please take a look at the attachment for an alternative
> implementation which includes an explanatory comment. YMMV. Feel free
> to ignore it.
>

+1. I find Peter's version with comments easier to understand.

thanks
Shveta