[PATCH] Several refactorings for pg_dump
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253472psql -h localhost -U postgresBuilt from patchset v4 (message #4), August 23, 2026 at 01:07 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253472_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253472_4 && git checkout t253472_4Patchset v4 (message #4) is on t253472_4
Hi,
While examining pg_dump code I found some room for improvement:
1. I found a write-only field ArchiveHandle.lookaheadSize which can
safely be removed.
2. I found magic constants 512 which should be replaced with TAR_BLOCK_SIZE.
Patches are attached.
--
Best regards,
Aleksander Alekseev
Attachments:
t253472_1v1-0001-pg_dump-remove-unused-ArchiveHandle.lookaheadSize.patchtext/x-patch; charset=US-ASCII; name=v1-0001-pg_dump-remove-unused-ArchiveHandle.lookaheadSize.patchDownload+0-3
v1-0002-pg_dump-use-TAR_BLOCK_SIZE-instead-of-a-hardcoded.patchtext/x-patch; charset=US-ASCII; name=v1-0002-pg_dump-use-TAR_BLOCK_SIZE-instead-of-a-hardcoded.patchDownload+5-5
On Tue, Aug 18, 2026 at 06:10:45PM +0300, Aleksander Alekseev wrote:
1. I found a write-only field ArchiveHandle.lookaheadSize which can
safely be removed.
2. I found magic constants 512 which should be replaced with TAR_BLOCK_SIZE.
Sounds like an acceptable set of cleanups here.
--
Michael
On 18 Aug 2026, at 17:10, Aleksander Alekseev <aleksander@tigerdata.com> wrote:
1. I found a write-only field ArchiveHandle.lookaheadSize which can
safely be removed.
2. I found magic constants 512 which should be replaced with TAR_BLOCK_SIZE.
Might be a nitpick, but I'm not sure that using TAR_BLOCK_SIZE is preferred for
a lookahead buffer used to identify the archive format where tar is just one of
the options.
I would be more inclined to initialize .lookaheadSize like today and then use
that instead of the hardcoded 512. Having a buffer in ArchiveHandle without
knowing the size of it does seem like a bad practice even if nothing uses it
today.
--
Daniel Gustafsson
Hi Daniel,
I'm not sure that using TAR_BLOCK_SIZE is preferred for
a lookahead buffer used to identify the archive format where tar is just one of
the options.I would be more inclined to initialize .lookaheadSize like today and then use
that instead of the hardcoded 512. Having a buffer in ArchiveHandle without
knowing the size of it does seem like a bad practice even if nothing uses it
today.
Sounds good. Here is the patch v2.
--
Best regards,
Aleksander Alekseev
On Thu, Aug 20, 2026 at 03:53:26PM +0300, Aleksander Alekseev wrote:
Sounds good. Here is the patch v2.
LGTM, at quick glance.
--
Michael
On 20 Aug 2026, at 14:53, Aleksander Alekseev <aleksander@tigerdata.com> wrote:
Hi Daniel,
I'm not sure that using TAR_BLOCK_SIZE is preferred for
a lookahead buffer used to identify the archive format where tar is just one of
the options.I would be more inclined to initialize .lookaheadSize like today and then use
that instead of the hardcoded 512. Having a buffer in ArchiveHandle without
knowing the size of it does seem like a bad practice even if nothing uses it
today.Sounds good. Here is the patch v2.
Seems reasonable. Are there any consumers of the buffer who hardcode an
assumption of its size which could use .lookaheadSize instead? I'll revist
this on Sunday when I get back in the office unless beaten to it.
--
Daniel Gustafsson
On Fri, Aug 21, 2026 at 08:28:52AM +0200, Daniel Gustafsson wrote:
Seems reasonable. Are there any consumers of the buffer who hardcode an
assumption of its size which could use .lookaheadSize instead? I'll revist
this on Sunday when I get back in the office unless beaten to it.
Please feel free to not wait for me. Thanks!
--
Michael