On Fri, Sep 25, 2026 at 10:21:26PM -0700, Nikhil Kumar Veldanda wrote:
> On Fri, Sep 25, 2026 at 8:25 PM Japin Li <[email protected]> wrote:
>> In the v4-0001 commit message, it says no behavior change.  However, it seems
>> an Assert introduced after setting vartag.
> 
> Fair.  The assertion cannot fire (both callers pass a constant tag)
> and is compiled out without --enable-cassert, but I'll mention it in
> the commit message next time.

I'm not really sure that this is worth mentioning at all.  You can
qualify that as a "change" because it adds one line, yes, but it's
just here to make sure that nobody does something stupid with the new
routine introduced by v4-0001, and the code is still the same as the
original version, just de-duplicated.

Applied this one.

>> Since the access/detoast.h include the access/toast_compression.h, should we 
>> remove
>> the access/toast_compression.h in files that already include access/toash.h?
> 
> I'd keep them.  All six files use toast_compression.h symbols
> directly, and the convention is to include what you use rather than
> rely on a transitive include (src/tools/pginclude/README).  detoast.h
> includes it for its own field, and e27f3b2cad7 added these includes to
> exactly these files for the same reason.

Hmm.  I'd say that this depends.  Cleanups of dependencies in .h files
are usually kind of nice, because it shows a cleaner chain of
dependencies at header level.  If you'd like to suggest a patch,
please feel free.  I am not sure how much this is worth it without
looking at some code shaping your idea.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to