On Fri, Jul 18, 2025 at 3:17 PM Noah Misch <noah@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
