Re: Dict issue with PostgreSQL for last_login plugin (duplicate key)

[John Fawcett via dovecot]

On 19/05/2019 22:37, John Fawcett via dovecot wrote:
> On 19/05/2019 20:31, mabi via dovecot wrote:
>>
>> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>> On Sunday, May 19, 2019 7:36 PM, John Fawcett via dovecot
>> <dovecot@dovecot.org> wrote:
>>>
>>> Attached is a tentative patch. I've verified no regression for
>>> mysql. There should be no regression for sqlite as the code path is
>>> identical.
>>>
>>> Are you able to test for pgsql? As mentioned by Akie it will break
>>> for PostgresSql < 9.5 but probably it was not working anyway due to
>>> duplicate keys. Whether this is a wider problem depends on whether
>>> the insert code is being used for other purposes too.
>>>
>>> If you or someone can verify it works on PostgresSql >= 9.5, then
>>> the next step will be to make it conditional on the version.
>>>
>> Thank you very much John for your patch, that's fantastic. I am on
>> OpenBSD 6.5 and will recompile dovecot from the ports by adding your
>> patch to it, I hope that works and will let you know if I managed. If
>> I understand correctly the relevant binary file I need to replace is
>> the following right:
>>
>> /usr/local/lib/dovecot/dict/libdriver_pgsql.so
>>
>> or are there any others I also need to replace in order to test? I am
>> planning to test live by just replacing the relevant file(s) so that
>> I hopefully don't need to re-install the whole dovecot package.
>
> I'm not sure how the source compilation works on OpenBSD, when I do it
> on linux and run "make install" it installs all relevant
> binaries/libraries.
>
> I saw one issue with the fix though, it does not correctly pull out
> the username field. I'm wondering if the query can be rewritten not to
> mention the name of the field that fails the constraint....
>
> John
>
so basically if this works just as well:

INSERT INTO last_logins (last_login,username,domain) VALUES
(1558273000,'u...@domain.tld <mailto:u...@domain.tld>','domain.tld') ON
CONFLICT DO UPDATE SET last_login=1558273000,domain='domain.tld';

then the fix can be altered to attached file which is more similar to
the MYSQL syntax and does not require extra logic to get the username field.

John




--- dict-sql-private.h.orig     2019-05-19 19:00:12.395887496 +0200
+++ dict-sql-private.h  2019-05-19 19:04:00.147601310 +0200
@@ -13,6 +13,7 @@
        HASH_TABLE(const char *, struct sql_prepared_statement *) 
prep_stmt_hash;
 
        bool has_on_duplicate_key:1;
+       bool has_on_conflict_do_update:1;
 };
 
 #endif
--- dict-sql.c.orig     2019-05-19 18:58:02.435194691 +0200
+++ dict-sql.c  2019-05-19 19:17:52.613253822 +0200
@@ -105,8 +105,10 @@
        i_zero(&sql_set);
        sql_set.driver = driver->name;
        sql_set.connect_string = dict->set->connect;
-       /* currently pgsql and sqlite don't support "ON DUPLICATE KEY" */
+       /* pgsql and sqlite don't support "ON DUPLICATE KEY" */
+       /* mysql and sqlite don't support "ON CONFLICT DO UPDATE" */
        dict->has_on_duplicate_key = strcmp(driver->name, "mysql") == 0;
+       dict->has_on_conflict_do_update = strcmp(driver->name, "pgsql") == 0;
 
        if (sql_db_cache_new(dict_sql_db_cache, &sql_set, &dict->db, error_r) < 
0) {
                pool_unref(&pool);
@@ -1108,12 +1110,15 @@
 
        str_append_str(prefix, suffix);
        str_append_c(prefix, ')');
-       if (!dict->has_on_duplicate_key) {
+       if (dict->has_on_duplicate_key ) {
+               str_append(prefix, " ON DUPLICATE KEY UPDATE ");
+       } else if(dict->has_on_conflict_do_update)  {
+               str_append(prefix, " ON CONFLICT DO UPDATE SET ");
+       } else {
                *stmt_r = sql_dict_transaction_stmt_init(ctx, str_c(prefix), 
&params);
                return 0;
        }
 
-       str_append(prefix, " ON DUPLICATE KEY UPDATE ");
        for (i = 0; i < field_count; i++) {
                const char *first_value_field =
                        t_strcut(fields[i].map->value_field, ',');