pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_dump sort priority mismatch for large objects
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2025-07-10 fb6c860bbd 36026b0fe3 e43fae199e pg_dump: Fix object-type sort priority for large objects.
* pg_dump sort priority mismatch for large objects
@ 2025-07-10 17:31 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Nathan Bossart @ 2025-07-10 17:31 UTC (permalink / raw)
To: Nitin Motiani <nitinmotiani@google.com>; +Cc: Hannu Krosing <hannuk@google.com>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
(moving to a new thread)
On Thu, Jul 10, 2025 at 06:05:26PM +0530, Nitin Motiani wrote:
>> There might be an existing issue here, because dbObjectTypePriorities
>> has the following comment:
>>
>> * NOTE: object-type priorities must match the section assignments made in
>> * pg_dump.c; that is, PRE_DATA objects must sort before DO_PRE_DATA_BOUNDARY,
>> * POST_DATA objects must sort after DO_POST_DATA_BOUNDARY, and DATA objects
>> * must sort between them.
>>
>> But dumpLO() puts large objects in SECTION_DATA, and PRIO_LARGE_OBJECT
>> is before PRIO_PRE_DATA_BOUNDARY. I admittedly haven't spent too much
>> time investigating this, though.
>
> I looked through the history of this to see how this happened and if it
> could be an existing issue. Prior to a45c78e3284b, dumpLO used to put large
> objects in SECTION_PRE_DATA. That commit changed dumpLO and also changed
> addBoundaryDependencies to move DO_LARGE_OBJECT from pre-data to data
> section. Seems like since then this has been inconsistent with
> pg_dump_sort.c. I think the change in pg_dump_sort.c should be backported
> to PG17 & 18 independent of the state of the larger patch.
Thanks for doing the archaeology!
> But even with the inconsistency, it doesn't look like there is an existing
> issue. As the dependencies were changed in addBoundaryDependencies, that
> should take precedence over the order in pg_dump_sort.c. The
> dbObjectTypePriorities are used by sortDumpableObjectsByTypeName. But right
> after that sortDumpableObjects sorts the objects based on dependencies
> therefore the change in boundary dependencies should ensure that this is
> working as intended. Still I think dbObjectTypePriorities should be made
> consistent with the rest.
+1, if for no other reason than we'll need it to be below PRIO_TABLE_DATA
for the speed-up-pg_upgrade-with-many-LOs patch [0]. Does anyone see any
problems with applying something like the following down to v17?
inline patch 1: src/bin/pg_dump/pg_dump_sort.c (+1/-1 lines) download inline-1.patch
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 0b0977788f1..538e7dcb493 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -76,10 +76,10 @@ enum dbObjectTypePriorities
PRIO_TABLE_ATTACH,
PRIO_DUMMY_TYPE,
PRIO_ATTRDEF,
- PRIO_LARGE_OBJECT,
PRIO_PRE_DATA_BOUNDARY, /* boundary! */
PRIO_TABLE_DATA,
PRIO_SEQUENCE_SET,
+ PRIO_LARGE_OBJECT,
PRIO_LARGE_OBJECT_DATA,
PRIO_STATISTICS_DATA_DATA,
PRIO_POST_DATA_BOUNDARY, /* boundary! */
* Re: pg_dump sort priority mismatch for large objects
@ 2025-07-10 17:38 Tom Lane <tgl@sss.pgh.pa.us>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2025-07-10 17:38 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Nitin Motiani <nitinmotiani@google.com>; Hannu Krosing <hannuk@google.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Jul 10, 2025 at 06:05:26PM +0530, Nitin Motiani wrote:
>> I looked through the history of this to see how this happened and if it
>> could be an existing issue. Prior to a45c78e3284b, dumpLO used to put large
>> objects in SECTION_PRE_DATA. That commit changed dumpLO and also changed
>> addBoundaryDependencies to move DO_LARGE_OBJECT from pre-data to data
>> section. Seems like since then this has been inconsistent with
>> pg_dump_sort.c. I think the change in pg_dump_sort.c should be backported
>> to PG17 & 18 independent of the state of the larger patch.
> +1, if for no other reason than we'll need it to be below PRIO_TABLE_DATA
> for the speed-up-pg_upgrade-with-many-LOs patch [0]. Does anyone see any
> problems with applying something like the following down to v17?
That's clearly an oversight in a45c78e3284b. I agree that fixing
pg_dump_sort.c to match shouldn't create any functional difficulties.
It might make the topological sort step marginally faster by
reducing the number of ordering violations that have to be fixed.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pg_dump sort priority mismatch for large objects
@ 2025-07-10 20:57 Nathan Bossart <nathandbossart@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Bossart @ 2025-07-10 20:57 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Nitin Motiani <nitinmotiani@google.com>; Hannu Krosing <hannuk@google.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Committed/back-patched.
--
nathan
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-07-10 20:57 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-10 17:31 pg_dump sort priority mismatch for large objects Nathan Bossart <nathandbossart@gmail.com>
2025-07-10 17:38 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-10 20:57 ` Nathan Bossart <nathandbossart@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox