On Wed, Jan 28, 2026 at 11:00 PM Hannu Krosing <[email protected]> wrote:
>
> v13 has added a proper test comparing original and restored table data
>
I was reviewing v13 and here are some initial comments I have
1. IMHO the commit message details about the work progress instead of
a high level idea about what it actually does and how.
Suggestion:
SUBJECT: Add --max-table-segment-pages option to pg_dump for parallel
table dumping.
This patch introduces the ability to split large heap tables into segments
based on a specified number of pages. These segments can then be dumped in
parallel using the existing jobs infrastructure, significantly reducing
the time required to dump very large tables.
The implementation uses ctid-based range queries (e.g., WHERE ctid >=
'(start,1)'
AND ctid <= '(end,32000)') to extract specific chunks of the relation.
<more architecture details and limitation if any>
2.
+ pg_log_warning("CHUNKING: set dopt.max_table_segment_pages to [%u]",
dopt.max_table_segment_pages);
+ break;
IMHO we don't need to place warning here while processing the input parameters
3.
+ printf(_(" --max-table-segment-pages=NUMPAGES\n"
+ " Number of main table pages
above which data is \n"
+ " copied out in chunks, also
determines the chunk size\n"));
Check the comment formatting, all the parameter description starts
with lower case, so better we start with "number" rather than "Number"
4.
+ if (is_segment(tdinfo))
+ {
+ appendPQExpBufferStr(q, tdinfo->filtercond?" AND ":" WHERE ");
+ if(tdinfo->startPage == 0)
+ appendPQExpBuffer(q, "ctid <= '(%u,32000)'", tdinfo->endPage);
+ else if(tdinfo->endPage != InvalidBlockNumber)
+ appendPQExpBuffer(q, "ctid BETWEEN '(%u,1)' AND '(%u,32000)'",
+ tdinfo->startPage, tdinfo->endPage);
+ else
+ appendPQExpBuffer(q, "ctid >= '(%u,1)'", tdinfo->startPage);
+ pg_log_warning("CHUNKING: pages [%u:%u]",tdinfo->startPage, tdinfo->endPage);
+ }
IMHO we should explain this chunking logic in the comment above this code block?
--
Regards,
Dilip Kumar
Google
