domain_admins table needs PRIMARY KEY

[rootwyrm]

Try as I might, I can't seem to find the schema to send over a pull request. This is a pretty simple fix. In PostgreSQL, postfixadmin creates the domain_admins table thusly:

CREATE TABLE public.domain_admins (
    username character varying(255) NOT NULL,
    domain character varying(255) NOT NULL,
    created timestamp with time zone DEFAULT now(),
    active boolean DEFAULT true NOT NULL
);

Consequently, PUBLICATION/SUBSCRIPTION and pglogical based replication will fail on the domain_admins table. Which is fine for a PUB/SUB scenario where subscribers don't have their own postfixadmin interface. It's not so great if you're looking to actually HA the database backing postfixadmin. (I'm sure MySQL/MariaDB has the same issue.)

Fix is to change the domain_admins table to this:

CREATE TABLE public.domain_admins (
    username character varying(255) NOT NULL PRIMARY KEY,
    domain character varying(255) NOT NULL,
    created timestamp with time zone DEFAULT now(),
    active boolean DEFAULT true NOT NULL
);

[DavidGoodwin]

MySQL doesn't have any concept of PubSub etc :) (at least not that I've come across yet, I've not looked at 8.0 though).

I don't think we can add a primary key on username there, as you might have e.g.

MariaDB [postfixadmin]> select * from domain_admins;
+------------------------+-------------+---------------------+--------+
| username               | domain      | created             | active |
+------------------------+-------------+---------------------+--------+
| pickle@gmail.com       | example.com | 2021-01-26 21:33:53 |      1 |
| david@example.com      | example.com | 2021-01-26 21:34:32 |      1 |
| david@example.com      | example.org | 2021-01-26 21:34:32 |      1 |
| david@example.org      | ALL         | 2021-02-24 21:30:38 |      1 |
+------------------------+-------------+---------------------+--------+
5 rows in set (0.001 sec)

[rootwyrm]

Ah, that's a good point, I didn't think of that issue. Having created as the PRIMARY KEY would serve the same function here then, since the crucial piece is having a PRIMARY KEY. What the key is doesn't matter.

MySQL/MariaDB is a bit more complicated of a situation. It's a non-issue on MariaDB because they have enhancements to ROW based replication - but an INDEX is required. However, on MySQL and other variants, the absence of a PRIMARY KEY will cause ROW based replication to break. So the PRIMARY KEY answer is probably best.

edit: Hmm, that's also an interesting quirk. On PgSQL, created is timestamp with tz. Based on your example, MySQL doesn't appear to be with timezone. Should that also be normalized?

[DavidGoodwin]

hm, all the datetime fields in PostfixAdmin are 'datetime' with no timezone specified. I think they're always set with the current date time from the server, and not exposed to end users (as such), so it'd normally be down to whatever time the server returns ? Perhaps this isn't ideal, but at least for me it does seem to use my current time :) (BST)

[neuffer]

Couldn't you just add an integer identity (auto increment) column? That would work fine as a primary key.
I'm sure Mysql/MariaDB has something similar.
With the timestamp you can not be sure not to get collisions.

[rootwyrm]

hm, all the datetime fields in PostfixAdmin are 'datetime' with no timezone specified.

I'm talking purely about the storage type in the schema. The default is always now(). Really timestamptz is still UTC internally, just translated on query.

With the timestamp you can not be sure not to get collisions.

Er, yes, you can. Because it uses now(). So the actual precision in PgSQL is to six places (microseconds.) An actual entry would be:
ALL 0 0 0 0 f 2020-05-15 15:53:31.914473-04 2020-05-15 15:53:31.914473-04 t

15:53:31.914473-04 will never appear twice, because of the way things operate internally when using now(). You would have 15:53:31.914474 or later, even for a parallel. You would have to manually insert a timestamp, which postfixadmin doesn't do.

The problem here is that MySQL and MariaDB TIMESTAMP is not equivalent by default. It does have microsecond precision and also stores in UTC. But now() is only second precision. To get identical behavior, MySQL needs to use now(6) (now with precision 6.)

A quick demonstration:

postgresql
template1=# SELECT now();
              now
-------------------------------
 2021-04-13 13:05:39.386501-04

mariadb105
root@localhost [(none)]> select now(); select now(6);
+---------------------+
| now()               |
+---------------------+
| 2021-04-13 13:07:13 |
+---------------------+
1 row in set (0.000 sec)

+----------------------------+
| now(6)                     |
+----------------------------+
| 2021-04-13 13:07:13.518640 |
+----------------------------+
1 row in set (0.000 sec)

However, I definitely agree that a sequence is a better solution - especially since I missed that gateways also has no PKEY. Generally speaking, every single table should have a PRIMARY KEY just as a matter of good hygiene. (Yes, I know there's a whole debate about that, so just my opinion.) My concern there would be the impact of the schema change on existing installations.

[DavidGoodwin]

ah, yes.... a sequence is the obvious answer (and best).....

[rootwyrm]

Yep... you can tell I hadn't had enough coffee. (Teach me to dredge up my old undocumented stuff, right? 🤦)

The problem is, I can't find where the heck it's even creating domain_admins. That said, the proposed fix is pretty simple. If I could find where it is. ;( There's no need to modify any queries, only the schema since it's just a throwaway sequence to fix replication.

CREATE SEQUENCE public.domain_admins_seq
    INCREMENT 1
    START 1
    CACHE 1;

CREATE TABLE public.domain_admins (
    id integer NOT NULL,
    username character varying(255) NOT NULL,
    domain character varying(255) NOT NULL,
    created timestamp with time zone DEFAULT now(),
    active boolean DEFAULT true NOT NULL
);

ALTER TABLE ONLY public.domain_admins ALTER COLUMN id SET DEFAULT nextval('public.domain_admins_seq'::regclass);

[DavidGoodwin]

many changesets later ....
the fix is

postfixadmin/public/upgrade.php

Line 2010 in af2cba2

function upgrade_1844()

- which ignores the 'id' field for sqlite.

[rootwyrm]

I can confirm after applying @DavidGoodwin 's commits by hand, both PUBLICATION/SUBSCRIPTION and pglogical replication (including bidirectional) of the database is now functional out of the box.