On Fri, Jul 18, 2025 at 3:17 PM Noah Misch <n...@leadboat.com> wrote: > > This comment is useful, but if I were to be critical, it does a better > > job saying what this field isn't than what it is. > > True. I've changed it to this:
That looks great. > - /* To have a stable sort order, break ties for some object types */ > + /* > + * To have a stable sort order, break ties for some object types. > Most > + * catalogs have a natural key, e.g. pg_proc_proname_args_nsp_index. > + * Where the above "namespace" and "name" comparisons don't cover all > + * natural key columns, compare the rest here. > + * > + * The natural key usually refers to other catalogs by surrogate keys. > + * Hence, this translates each of those references to the natural key > of > + * the referenced catalog. That may descend through multiple levels > of > + * catalog references. For example, to sort by pg_proc.proargtypes, > + * descend to each pg_type and then further to its pg_namespace, for > an > + * overall sort by (nspname, typname). > + */ I really like this. > + * Sort by encoding, per pg_collation_name_enc_nsp_index. > Wherever > + * this changes dump order, restoring the dump fails anyway. > CREATE > + * COLLATION can't create a tie for this to break, because it > imposes > + * restrictions to make (nspname, collname) uniquely identify > a > + * collation within a given DatabaseEncoding. While > + * pg_import_system_collations() can create a tie, > pg_dump+restore > + * fails after pg_import_system_collations('my_schema') does > so. > + * There's little to gain by ignoring one natural key column > on the > + * basis of those limitations elsewhere, so respect the full > natural > + * key like we do for other object types. This is also good. I suggest s/Wherever/Technically, this is not necessary, because wherever/ and s/There's/However, there's/. -- Robert Haas EDB: http://www.enterprisedb.com