[IMP] convert PG errors through pg93 diag info

Previously availability of meta-information (failing constraint,
impacted table and column, …) in pg errors (e.g. constraint check
failure) was limited and only available through formatted error
messages, which could be localised (by PG itself) so not did getting
that information require string extraction it was brittle in the face of
localised instances.

Postgres 9.3 adds this meta-information to error diagnostic data
(`PQresultErrorField`), allowing easy programmatic access to it.
Psycopg2 [added a Diagnostic
object](http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Diagnostics)
around the time pg9.3 was itself released.

Assuming Odoo now requires pg >= 9.3 we can remove the old string
munging extraction for diagnostic.

* funny story, the handling of 23505 didn't actually work because
  "duplicate key value violates unique constraint" is a literal part
  of the error message, I'd misunderstood "value" as a field name
  because my test model's field was called "value". Makes sense too
  since you can set UNIQUE constraints on multiple fields (and UNIQUE
  indices on expressions)
* for both errors, it should be possible to use the table_name or
  constraint metadata to provide messages about sub-model issues (a
  constraint on an m2o), but that's not currently handled

Fixes #15323

3 files changed, 80 insertions, 31 deletions

diff --git a/odoo/addons/test_impex/models.py b/odoo/addons/test_impex/models.py
index 9dc1f6b31ed..f1e10b10abf 100644
--- a/odoo/addons/test_impex/models.py
+++ b/odoo/addons/test_impex/models.py
@@ -3,6 +3,7 @@
 
 from odoo import api, fields, models
 
+
 def selection_fn(model):
     return [(str(key), val) for key, val in enumerate(["Corge", "Grault", "Wheee", "Moog"])]
 
@@ -148,7 +149,10 @@ class OnlyOne(models.Model):
     _name = 'export.unique'
 
     value = fields.Integer()
+    value2 = fields.Integer()
+    value3 = fields.Integer()
 
     _sql_constraints = [
         ('value_unique', 'unique (value)', "The value must be unique"),
+        ('pair_unique', 'unique (value2, value3)', "The values must be unique"),
     ]
diff --git a/odoo/addons/test_impex/tests/test_load.py b/odoo/addons/test_impex/tests/test_load.py
index 807fa452448..d8c7e3d87d5 100644
--- a/odoo/addons/test_impex/tests/test_load.py
+++ b/odoo/addons/test_impex/tests/test_load.py
@@ -3,6 +3,7 @@
 
 import json
 import pkgutil
+import re
 
 from odoo.tests import common
 from odoo.tools.misc import mute_logger
@@ -1116,14 +1117,41 @@ class test_unique(ImporterCase):
         ])
         self.assertFalse(result['ids'])
         self.assertEqual(result['messages'], [
-            dict(message=u"The value for the field 'value' already exists. "
-                         u"This might be 'Value' in the current model, "
-                         u"or a field of the same name in an o2m.",
+            dict(message=u"The value for the field 'value' already exists "
+                         u"(this is probably 'Value' in the current model).",
                  type='error', rows={'from': 1, 'to': 1},
                  record=1, field='value'),
-            dict(message=u"The value for the field 'value' already exists. "
-                         u"This might be 'Value' in the current model, "
-                         u"or a field of the same name in an o2m.",
+            dict(message=u"The value for the field 'value' already exists "
+                         u"(this is probably 'Value' in the current model).",
                  type='error', rows={'from': 4, 'to': 4},
                  record=4, field='value'),
         ])
+
+    @mute_logger('odoo.sql_db')
+    def test_unique_pair(self):
+        result = self.import_(['value2', 'value3'], [
+            ['0', '1'],
+            ['1', '0'],
+            ['1', '1'],
+            ['1', '1'],
+        ])
+        self.assertFalse(result['ids'])
+        self.assertEqual(len(result['messages']), 1)
+        message = result['messages'][0]
+        self.assertEqual(message['type'], 'error')
+        self.assertEqual(message['record'], 3)
+        self.assertEqual(message['rows'], {'from': 3, 'to': 3})
+        m = re.match(
+            r"The values for the fields '([^']+)' already exist "
+            r"\(they are probably '([^']+)' in the current model\)\.",
+            message['message']
+        )
+        self.assertIsNotNone(m)
+        self.assertItemsEqual(
+            m.group(1).split(', '),
+            ['value2', 'value3']
+        )
+        self.assertItemsEqual(
+            m.group(2).split(', '),
+            ['Value2', 'Value3']
+        )
diff --git a/odoo/models.py b/odoo/models.py
index f0a5cb16786..8e88b82c788 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -30,6 +30,7 @@ import operator
 import pytz
 import re
 from collections import defaultdict, MutableMapping, OrderedDict
+from contextlib import closing
 from inspect import getmembers, currentframe
 from operator import attrgetter, itemgetter
 
@@ -5090,45 +5091,61 @@ def itemgetter_tuple(items):
         return lambda gettable: (gettable[items[0]],)
     return operator.itemgetter(*items)
 
-def convert_pgerror_23502(model, fields, info, e):
-    m = re.match(r'^null value in column "(?P<field>\w+)" violates '
-                 r'not-null constraint\n',
-                 tools.ustr(e))
-    field_name = m and m.group('field')
-    if not m or field_name not in fields:
+def convert_pgerror_not_null(model, fields, info, e):
+    if e.diag.table_name != model._table:
         return {'message': tools.ustr(e)}
-    message = _(u"Missing required value for the field '%s'.") % field_name
-    field = fields.get(field_name)
-    if field:
-        message = _(u"Missing required value for the field '%s' (%s)") % (field['string'], field_name)
+
+    field_name = e.diag.column_name
+    field = fields[field_name]
+    message = _(u"Missing required value for the field '%s' (%s)") % (field['string'], field_name)
     return {
         'message': message,
         'field': field_name,
     }
 
-def convert_pgerror_23505(model, fields, info, e):
-    m = re.match(r'^duplicate key (?P<field>\w+) violates unique constraint',
-                 tools.ustr(e))
-    field_name = m and m.group('field')
-    if not m or field_name not in fields:
+def convert_pgerror_unique(model, fields, info, e):
+    # new cursor since we're probably in an error handler in a blown
+    # transaction which may not have been rollbacked/cleaned yet
+    with closing(model.env.registry.cursor()) as cr:
+        cr.execute("""
+            SELECT
+                conname AS "constraint name",
+                t.relname AS "table name",
+                ARRAY(
+                    SELECT attname FROM pg_attribute
+                    WHERE attrelid = conrelid
+                      AND attnum = ANY(conkey)
+                ) as "columns"
+            FROM pg_constraint
+            JOIN pg_class t ON t.oid = conrelid
+            WHERE conname = %s
+        """, [e.diag.constraint_name])
+        constraint, table, ufields = cr.fetchone() or (None, None, None)
+    # if the unique constraint is on an expression or on an other table
+    if not ufields or model._table != table:
         return {'message': tools.ustr(e)}
-    message = _(u"The value for the field '%s' already exists.") % field_name
-    field = fields.get(field_name)
-    if field:
-        message = _(u"%s This might be '%s' in the current model, or a field "
-                    u"of the same name in an o2m.") % (message, field['string'])
+
+    # TODO: add stuff from e.diag.message_hint? provides details about the constraint & duplication values but may be localized...
+    if len(ufields) == 1:
+        field_name = ufields[0]
+        field = fields[field_name]
+        message = _(u"The value for the field '%s' already exists (this is probably '%s' in the current model).") % (field_name, field['string'])
+        return {
+            'message': message,
+            'field': field_name,
+        }
+    field_strings = [fields[fname]['string'] for fname in ufields]
+    message = _(u"The values for the fields '%s' already exist (they are probably '%s' in the current model).") % (', '.join(ufields), ', '.join(field_strings))
     return {
         'message': message,
-        'field': field_name,
+        # no field, unclear which one we should pick and they could be in any order
     }
 
 PGERROR_TO_OE = defaultdict(
     # shape of mapped converters
     lambda: (lambda model, fvg, info, pgerror: {'message': tools.ustr(pgerror)}), {
-    # not_null_violation
-    '23502': convert_pgerror_23502,
-    # unique constraint error
-    '23505': convert_pgerror_23505,
+    '23502': convert_pgerror_not_null,
+    '23505': convert_pgerror_unique,
 })
 
 def _normalize_ids(arg, atoms=set(IdType)):