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 <japinli@hotmail.com> 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
