[PATCH] Sort policies and triggers by table name in pg_dump.

On Mon, Sep 23, 2019 at 10:34:07PM +0100, Benjie Gillam wrote:

The attached draft patch (made against `pg_dump_sort.c` on master) breaks
ties for trigger and policy objects by using the table name, increasing the
sort order stability. I have compiled it and executed it against a number of
local databases and it behaves as desired.

Could you provide a simple example of schema (tables with some
policies and triggers), with the difference this generates for
pg_dump, which shows your point?

I am new to PostgreSQL contribution and my C-skills are rusty, so please let
me know if I can improve the patch, or if there are areas of PostgreSQL that
I have overlooked.

Your patch has two warnings because you are trying to map a policy
info pointer to a trigger info pointer:
pg_dump_sort.c:224:24: warning: initialization of ‘TriggerInfo *’ {aka
‘struct _triggerInfo *’} from incompatible pointer type ‘PolicyInfo *
const’ {aka ‘struct _policyInfo * const’}
[-Wincompatible-pointer-types]
224 | TriggerInfo *tobj2 = *(PolicyInfo *const *) p2;
--
Michael

Could you provide a simple example of schema (tables with some
policies and triggers), with the difference this generates for
pg_dump, which shows your point?

Certainly; I've attached a bash script that can reproduce the issue
and the diff that it produces, here's the important part:

CREATE TRIGGER a BEFORE INSERT ON foo
FOR EACH ROW EXECUTE PROCEDURE qux();
CREATE POLICY a ON foo FOR SELECT USING (true);

CREATE TRIGGER a BEFORE INSERT ON bar
FOR EACH ROW EXECUTE PROCEDURE qux();
CREATE POLICY a ON bar FOR SELECT USING (true);

Here we create two identically named triggers and two identically
named policies on tables foo and bar. If instead we ran these
statements in a different order (or if the object IDs were to wrap)
the order of the pg_dump would be different even though the
databases are identical other than object IDs. The attached
patch eliminates this difference.

Your patch has two warnings because you are trying to map a policy
info pointer to a trigger info pointer:

Ah, thank you for the pointer (aha); I've attached an updated patch
that addresses this copy/paste issue.

On Tue, Sep 24, 2019 at 08:48:33AM +0100, Benjie Gillam wrote:

Here we create two identically named triggers and two identically
named policies on tables foo and bar. If instead we ran these
statements in a different order (or if the object IDs were to wrap)
the order of the pg_dump would be different even though the
databases are identical other than object IDs. The attached
patch eliminates this difference.

Thanks. Perhaps you could add your patch to the next commit fest
then at https://commitfest.postgresql.org/25/?

This way, your patch gains more visibility for potential reviews.
Another key thing to remember is that one patch authored requires one
other patch of equal difficulty to be reviewed.
--
Michael

Thanks. Perhaps you could add your patch to the next commit fest
then at https://commitfest.postgresql.org/25/?

Thanks, submitted.

Benjie Gillam <benjie@jemjie.com> writes:

Your patch has two warnings because you are trying to map a policy
info pointer to a trigger info pointer:

Ah, thank you for the pointer (aha); I've attached an updated patch
that addresses this copy/paste issue.

LGTM, pushed (with a bit of extra polishing of comments).

regards, tom lane