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


Reply via email to