MIME-Version: 1.0
References: <CAFjYY++22tmGDSac_YPA_NLJ4-cEJo7kS3SxXFHf7ZFTnwO-QA@mail.gmail.com>
 <CAFjYY+J7ZcJAoE+K4AoyuiS_N6w8vVCQ6HQ72mPB=i4C+QbAeQ@mail.gmail.com>
 <CAFjYY+KhiuzocMdgEr8wQtCkSaec=Mu_YiLE6N3JfYtQ-Xv=ug@mail.gmail.com>
 <CAFjYY++YjtunZcFt7MFBWwseP_=GS11=V36_xig07enHA0eRjA@mail.gmail.com>
 <CAPpHfdvD-oqh=qvtkD3qWqZrfxt0+PtdwyRYC+faQd-rr+V0mg@mail.gmail.com>
 <7bcc0420-d457-4af5-a459-a4e5d929a665@vondra.me> <CAFjYY+Lhw2tvsiC8_Dbx+K9dMaK=myT5Cck2=8FgLUW5f9Hp-A@mail.gmail.com>
 <udbqvyuxlxk4a64uomo5xmgloemk7jrbeeluhvxbdqxcdv2pt5@who34o73ttmo>
 <CAFjYY+LcBCPe9cboef1GteAPKEiqyTTww7NGxa==roc6QXSOxA@mail.gmail.com>
 <CAFjYY+LJ5_j82PrSxm2RjM0OjwfN9smJ1VKKMOka-25Jg-0ofg@mail.gmail.com>
 <7gbzkoxrpqvj2sgxof4pqirz7pf6wb5vizak3mxrz2zpvrf3pa@lvnwdsy3em56>
 <CAFjYY+LH=MbMpGiYfMtPsANDBAqiDC66hD3tBs_JeVcBYVcmHA@mail.gmail.com>
 <CAFjYY+Jfsy1r4ZP4+RyHZeWE7JyT4LSdVFvA9ck04LFkKd1ebw@mail.gmail.com>
 <CAFjYY+K=s8gL=3SN8puwjSm5ZvUXumsdeB-Z2rwLP5OQNtEF+Q@mail.gmail.com>
 <CAEvyyThUp+Cyv8Se6dxpJvP7w=u1670Xd4k6zeXGVSac7UOm_A@mail.gmail.com>
 <CAFjYY+KjoNLYGFeM4z=DOgG1uDwXD1Z6zyurgz1d9Y-96X=9GA@mail.gmail.com> <CAFjYY+JDSpOQwYAfTQQ43=BA=d32XfcAdaPVJgHheV9fQBbLWg@mail.gmail.com>
In-Reply-To: <CAFjYY+JDSpOQwYAfTQQ43=BA=d32XfcAdaPVJgHheV9fQBbLWg@mail.gmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Mon, 19 Jan 2026 22:40:16 +0000
Message-ID: <CAN4CZFPmAwOvR8r1gtv=r+4akArZJyjna_Nxgkn_eTcsRs4XPA@mail.gmail.com>
Subject: Re: Proposal: Adding compression of temporary files
To: Filip Janus <fjanus@redhat.com>
Cc: lakshmi <lakshmigcdac@gmail.com>, Tomas Vondra <tv@fuzzy.cz>, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAN4CZFPmAwOvR8r1gtv%3Dr%2B4akArZJyjna_Nxgkn_eTcsRs4XPA%40mail.gmail.com>
Precedence: bulk

Hello!

I tried to review the code. It compiled, the test suite passed.

I noticed two typos:

buffile.c:77 - "Disaled"
buffile.c:133 - "mathods"

And a few other small findings:

buffile.h:35 and buffile.c:63 - same constants defined first as an
Enum and then as #defines - code builds properly without the defines.

buffile.c:121 - compress_tempfile is defined, set to false at :167,
but never used otherwise

guc_tables.c:470 - the comment says that pglz isn't supported yet, but
we have a value for it, and I see support for it in the code

buffile.c:659: (and at other places) if USE_LZ4 is undefined, the
codepath doesn't do anything. I think these ifdefs should follow how
other compression code works, such as wal compression where there's an
#else path with elog(ERROR, ...)
Similarly, maybe there should be an explicit TEMP_NONE_COMPRESSION
branch that does nothing, and the default branch should be an error?

buffile.c:265: If seek isn't supported/limited, shouldn't there be at
least an assertion about it in BufFileSeek? And tell isn't mentioned,
but it seems to me that tell also doesn't work properly.