| 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.
| 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 |