From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | michael(at)paquier(dot)xyz, pg(at)bowt(dot)ie, melanieplageman(at)gmail(dot)com, boekewurm+postgres(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Add pg_walinspect function with block info columns |
Date: | 2023-03-27 04:11:01 |
Message-ID: | CALj2ACVWXOQK2HScKj+mFkxHbFaui3tH95cmZ3+hX=insNwKPg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 27, 2023 at 9:11 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
> > On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > OUT reltablespace oid,
> > OUT reldatabase oid,
> > OUT relfilenode oid,
> > OUT relblocknumber int8,
> > + OUT blockid int2,
> > + OUT start_lsn pg_lsn,
> > + OUT end_lsn pg_lsn,
> > + OUT prev_lsn pg_lsn,
> >
> > I'd still put the LSN data before the three OIDs for consistency with
> > the structures, though my opinion does not seem to count much..
>
> I agree with Michael on this point. Also, although it may not be
> significant for SQL, the rows are output in lsn order from the
> function.
Done that way.
On Sat, Mar 25, 2023 at 8:42 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > Please see the attached v4 patch set addressing all the review comments.
>
> - desc = GetRmgr(XLogRecGetRmid(record));
> - id = desc.rm_identify(XLogRecGetInfo(record));
> -
> - if (id == NULL)
> - id = psprintf("UNKNOWN (%x)", XLogRecGetInfo(record) & ~XLR_INFO_MASK);
> -
> - initStringInfo(&rec_desc);
> - desc.rm_desc(&rec_desc, record);
> -
> - /* Block references. */
> - initStringInfo(&rec_blk_ref);
> - XLogRecGetBlockRefInfo(record, false, true, &rec_blk_ref, &fpi_len);
> -
> - main_data_len = XLogRecGetDataLen(record);
>
> I don't see any need to move this block of code? This leads to
> unnecessary diffs, potentially making backpatch a bit harder. Either
> way is not a big deal, still.. Except for this bit, 0001 looks fine
> by me.
It's a cosmetic change - I wanted to keep the calculation of column
values closer to where they're assigned to Datum values. I agree to
not cause too much diff and removed them.
Please see the attached v5 patch set.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Few-optimizations-in-pg_walinspect.patch | application/x-patch | 3.9 KB |
v5-0002-Emit-WAL-record-info-via-pg_get_wal_block_info.patch | application/x-patch | 12.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-03-27 04:18:57 | Re: Add pg_walinspect function with block info columns |
Previous Message | vignesh C | 2023-03-27 03:57:48 | Re: Support logical replication of DDLs |