From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Subject: | Re: Show various offset arrays for heap WAL records |
Date: | 2023-04-11 21:29:40 |
Message-ID: | CAAKRu_Z7CsF-KNGhgd-1oD-1q2pYQQm_7fSMns=J42bdWK_2Mw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Apr 11, 2023 at 1:35 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Tue, Apr 11, 2023 at 7:40 AM Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
> > Not the fault of this patch, but I also noticed that heap UPDATE and
> > HOT_UPDATE records have xmax twice and don't differentiate between new
> > and old. I think that was probably a mistake.
> >
> > description | off: 119, xmax: 1105, flags: 0x00, old_infobits:
> > [], new off: 100, xmax 0
>
> That doesn't seem great to me either. I don't like this ambiguity,
> because it seems like it makes the description hard to parse in a way
> that flies in the face of what we're trying to do here, in general.
> So it seems like it might be worth fixing now, in the scope of this
> patch.
Agreed.
On Tue, Apr 11, 2023 at 3:22 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Tue, Apr 11, 2023 at 11:48 AM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> > Attached revision deals with this by spelling out the names in full
> > (e.g., "old_xmax" and "new_xmax"). It also reorders the output fields
> > to match the order from the physical UPDATE, HOT_UPDATE, and LOCK WAL
> > record types, on the theory that those should match the physical
> > record (unless there is a good reason not to, which doesn't apply
> > here).
>
> I just noticed that we don't even show xmax in the case of DELETE
> records. Perhaps the original assumption is that it must match the
> record's own XID, but that's not true after the MultiXact enhancements
> for foreign key locking added to 9.3 (and in any case there is no
> reason at all to show xmax in UPDATE but not in DELETE).
>
> Attached revision v4 fixes this, making DELETE, UPDATE, HOT_UPDATE,
> LOCK, and LOCK_UPDATED record types consistent with each other in
> terms of the key names output by the heap desc routine. The field
> order also needed a couple of tweaks for struct consistency (and
> cross-record consistency) for v4.
Code in v4 all seems fine to me.
I like the update guidelines comment.
I agree it would be nice for xl_heap_lock->locking_xid to be renamed
xmax for clarity. I would suggest that if you don't intend to put it
in a separate commit, you mention it explicitly in the final commit
message. Its motivation isn't immediately obvious to the reader.
- Melanie
From | Date | Subject | |
---|---|---|---|
Next Message | samay sharma | 2023-04-11 21:41:38 | Re: Documentation for building with meson |
Previous Message | David Kimura | 2023-04-11 21:28:32 | Unexpected (wrong?) result querying boolean partitioned table with NULL partition |