Dict issue with PostgreSQL for last_login plugin (duplicate key)

[John Fawcett]

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 at 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,'user at domain.tld <mailto:user at 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



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://dovecot.org/pipermail/dovecot/attachments/20190519/b0ab52cc/attachment.html>
-------------- next part --------------
--- 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, ',');