dovecot - May 2019 - Dict issue with PostgreSQL for last_login plugin (duplicate key)

On 19/05/2019 16:25, John Fawcett via dovecot wrote:
> INSERT INTO last_logins (last_login,username,domain) VALUES
(1558273000,'user at domain.tld','domain.tld')
I don't have PostgresSql, would you be able to verify if this syntax would
work:

INSERT INTO last_logins (last_login,username,domain) VALUES
(1558273000,'user at domain.tld','domain.tld')
ONCONFLICT(username) UPDATE SET last_login=1558273000,domain='user at
domain.tld'

It's important to check that this updates only the single row for that user
and it puts the right data in that row. If it doesn't work can you give the
correct syntax?

John 

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://dovecot.org/pipermail/dovecot/attachments/20190519/9f6aa34b/attachment.html>

??????? Original Message ???????
On Sunday, May 19, 2019 4:44 PM, John Fawcett via dovecot <dovecot at
dovecot.org> wrote:
> I don't have PostgresSql, would you be able to verify if this syntax
would work: INSERT INTO last_logins (last_login,username,domain) VALUES
(1558273000,'user at domain.tld','domain.tld') ON
CONFLICT(username) UPDATE SET last_login=1558273000,domain='user at
domain.tld' It's important to check that this updates only the single
row for that user and it puts the right data in that row. If it doesn't work
can you give the correct syntax?
So you nearly yes ;-) The only parameter missing was "DO" keyword
before the "UPDATE". So the correct query would be:

INSERT INTO last_logins (last_login,username,domain) VALUES
(1558273000,'user at domain.tld','domain.tld') ON CONFLICT
(username) DO UPDATE SET last_login=1558273000,domain='domain.tld';

I also adapted the domain='domain.tld' at the end of the query, you had
domain='user at domain.tld' but this is just a "content"
detail which does not matter.

Hope that helps. Let me know if I can do any further testing.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://dovecot.org/pipermail/dovecot/attachments/20190519/78faf344/attachment.html>

On 19/05/2019 17:08, mabi via dovecot wrote:
> ??????? Original Message ???????
> On Sunday, May 19, 2019 4:44 PM, John Fawcett via dovecot
> <dovecot at dovecot.org> wrote:
>
>> I don't have PostgresSql, would you be able to verify if this
syntax
>> would work: INSERT INTO last_logins (last_login,username,domain)
>> VALUES (1558273000,'user at domain.tld
>> <mailto:user at domain.tld>','domain.tld') ON
CONFLICT(username) UPDATE
>> SET last_login=1558273000,domain='user at domain.tld
>> <mailto:user at domain.tld>' It's important to check that
this updates
>> only the single row for that user and it puts the right data in that
>> row. If it doesn't work can you give the correct syntax?
>
> So you nearly yes ;-) The only parameter missing was "DO" keyword
> before the "UPDATE". So the correct query would be:
>
> INSERT INTO last_logins (last_login,username,domain) VALUES
> (1558273000,'user at domain.tld <mailto:user at
domain.tld>','domain.tld')
> ON CONFLICT (username) DO UPDATE SET
> last_login=1558273000,domain='domain.tld';
>
> I also adapted the domain='domain.tld' at the end of the query, you
> had domain='user at domain.tld <mailto:user at domain.tld>'
but this is just
> a "content" detail which does not matter.
>
> Hope that helps. Let me know if I can do any further testing.
>
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.

John

John

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<https://dovecot.org/pipermail/dovecot/attachments/20190519/cfd5cdd2/attachment-0001.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_printfa(prefix, " ON CONFLICT (%s) DO UPDATE SET
",fields[0].map->username_field);
+	} 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,
',');