<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 21/05/2019 15:45, mabi via dovecot
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:mbewHDmkKnfvirPIoD-rVmNKlSTsV5oz3BKG-Va9pGNCrXI06R-IzVRq4gLE1l5VYvTcD63wxh4d7tbi_9y6X5ToFqtn4XCQFq_no_l2qnI=@protonmail.ch">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div>‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐<br>
</div>
<div> On Monday, May 20, 2019 12:37 AM, John Fawcett via dovecot
<a class="moz-txt-link-rfc2396E" href="mailto:dovecot@dovecot.org"><dovecot@dovecot.org></a> wrote:<br>
</div>
<div> <br>
</div>
<blockquote class="protonmail_quote" type="cite">
<div class="moz-cite-prefix">So looking into this with a
postgresql databse to work with: the above query does not
work. You have to specify either the column name or the
constraint name that you expect to be violated in order for
the update to take place.<br>
</div>
<p>With a map like this one you're using<br>
</p>
<pre><code>map {
pattern = shared/last-login/$user/$domain
table = last_login
value_field = last_login
value_type = uint
fields {
username = $user
domain = $domain
}
}</code>
</pre>
<p>there's no field name that is obviously the primary key. I've
reworked the patch to use the postgres default primary key
constraint name (tablename_pkey). <br>
</p>
</blockquote>
<div>So as you mention the new query you adapted which includes
the primary key works, I tested it manually against PostgreSQL
10.5.<br>
</div>
<blockquote class="protonmail_quote" type="cite">
<p>The attached fix should work in that case, although I feel
it's not general enough.<br>
</p>
</blockquote>
<div>Unfortunately my compiling skills are quite poor and I did
not manage to patch and recompile Dovecot on OpenBSD.<br>
</div>
<div><br>
</div>
<div>Do you think your patch will make it into the Dovecot code?<br>
</div>
<div><br>
</div>
</blockquote>
<p>I feel confident that the patch works as the query has been
manually verified and the code change is not complex to validate.</p>
<p>The last_login plugin does not work at the moment with PostgreSql
and probably does not work with Sqlite, given that the only logic
that tries an update when insert fails seems to be a MySQL
specific extension to standard Sql. So I think that it's clear
that support for PostgreSql and Sqlite needs to be implemented.
The same issue likely exist in other plugins too, for example
expire.<br>
</p>
<p>My doubts are around the right solution to adopt. Initially I
thought that there was a PostgreSql syntax similar to MySQL which
could be easily added to the code, but closer inspection shows
that the PostgreSql syntax requires specification of either a
constraint name or the index column(s) for the primary/unique
keys. <br>
</p>
<p>Constraint names are nowhere specified in the dictionary map
syntax and it's not possible either to identify with 100%
certainty the primary key column(s).</p>
<p>The solution I adopted in the latest version of the patch is to
use the default primary key constraint name derived from the table
name, but that won't help if people define custom constraint
names. That may be an unlikely scenario so the fix is certainly
better than AS-IS. However it is not perfect and added to that is
the fact that the PostgreSql extension is available only from 9.5.
<br>
</p>
<p>I have no issues to submit the patch officially, as long as
Dovecot developers agree. However it may be worthwhile reflecting
on a more structural change</p>
<p>1) logic which always tries to update and falls back to insert if
the update fails (or viceversa) for all sql dictionaries.</p>
<p>2) updates to the map syntax so that either the constraint name
or primary key columns can be specified.</p>
<p>Ideas are welcome.<br>
</p>
<p>John<br>
</p>
<p><br>
</p>
</body>
</html>