Re: Proposal: Adding compression of temporary files

From: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>
To: Filip Janus <fjanus(at)redhat(dot)com>
Cc: lakshmi <lakshmigcdac(at)gmail(dot)com>, Tomas Vondra <tv(at)fuzzy(dot)cz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Proposal: Adding compression of temporary files
Date: 2026-01-19 22:40:16
Message-ID: CAN4CZFPmAwOvR8r1gtv=r+4akArZJyjna_Nxgkn_eTcsRs4XPA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mihail Nikalayeu 2026-01-19 22:59:41 Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Previous Message Zsolt Parragi 2026-01-19 21:15:33 Re: Patch: dumping tables data in multiple chunks in pg_dump