Fix Record.keys() and Record.items() to correctly handle duplicate field names.

asyncpg/protocol/record/recordobj.c

@@ -455,7 +455,7 @@ record_keys(PyObject *o, PyObject *args)
         return NULL;
     }
 
-    return PyObject_GetIter(((ApgRecordObject*)o)->desc->mapping);
+    return PyObject_GetIter(((ApgRecordObject*)o)->desc->keys);
 }
 
 
@@ -688,7 +688,7 @@ record_iter(PyObject *seq)
 typedef struct {
     PyObject_HEAD
     Py_ssize_t it_index;
-    PyObject *it_map_iter;
+    PyObject *it_key_iter;
     ApgRecordObject *it_seq; /* Set to NULL when iterator is exhausted */
 } ApgRecordItemsObject;
 
@@ -697,7 +697,7 @@ static void
 record_items_dealloc(ApgRecordItemsObject *it)
 {
     PyObject_GC_UnTrack(it);
-    Py_CLEAR(it->it_map_iter);
+    Py_CLEAR(it->it_key_iter);
     Py_CLEAR(it->it_seq);
     PyObject_GC_Del(it);
 }
@@ -706,7 +706,7 @@ record_items_dealloc(ApgRecordItemsObject *it)
 static int
 record_items_traverse(ApgRecordItemsObject *it, visitproc visit, void *arg)
 {
-    Py_VISIT(it->it_map_iter);
+    Py_VISIT(it->it_key_iter);
     Py_VISIT(it->it_seq);
     return 0;
 }
@@ -726,11 +726,11 @@ record_items_next(ApgRecordItemsObject *it)
         return NULL;
     }
     assert(ApgRecord_CheckExact(seq));
-    assert(it->it_map_iter != NULL);
+    assert(it->it_key_iter != NULL);
 
-    key = PyIter_Next(it->it_map_iter);
+    key = PyIter_Next(it->it_key_iter);
     if (key == NULL) {
-        /* likely it_map_iter had less items than seq has values */
+        /* likely it_key_iter had less items than seq has values */
         goto exhausted;
     }
 
@@ -740,7 +740,7 @@ record_items_next(ApgRecordItemsObject *it)
         Py_INCREF(val);
     }
     else {
-        /* it_map_iter had more items than seq has values */
+        /* it_key_iter had more items than seq has values */
         Py_DECREF(key);
         goto exhausted;
     }
@@ -757,7 +757,7 @@ record_items_next(ApgRecordItemsObject *it)
     return tup;
 
 exhausted:
-    Py_CLEAR(it->it_map_iter);
+    Py_CLEAR(it->it_key_iter);
     Py_CLEAR(it->it_seq);
     return NULL;
 }
@@ -823,23 +823,23 @@ static PyObject *
 record_new_items_iter(PyObject *seq)
 {
     ApgRecordItemsObject *it;
-    PyObject *map_iter;
+    PyObject *key_iter;
 
     if (!ApgRecord_CheckExact(seq)) {
         PyErr_BadInternalCall();
         return NULL;
     }
 
-    map_iter = PyObject_GetIter(((ApgRecordObject*)seq)->desc->mapping);
-    if (map_iter == NULL) {
+    key_iter = PyObject_GetIter(((ApgRecordObject*)seq)->desc->keys);
+    if (key_iter == NULL) {
         return NULL;
     }
 
     it = PyObject_GC_New(ApgRecordItemsObject, &ApgRecordItems_Type);
     if (it == NULL)
         return NULL;
 
-    it->it_map_iter = map_iter;
+    it->it_key_iter = key_iter;
     it->it_index = 0;
     Py_INCREF(seq);
     it->it_seq = (ApgRecordObject *)seq;

tests/test_record.py

@@ -294,10 +294,33 @@ def test_record_empty(self):
 
     async def test_record_duplicate_colnames(self):
         """Test that Record handles duplicate column names."""
-        r = await self.con.fetchrow('SELECT 1 as a, 2 as a')
-        self.assertEqual(r['a'], 2)
-        self.assertEqual(r[0], 1)
-        self.assertEqual(repr(r), '<Record a=1 a=2>')
+
+        records_descs = [
+            [('a', 1)],
+            [('a', 1), ('a', 2)],
+            [('a', 1), ('b', 2), ('a', 3)],
+            [('a', 1), ('b', 2), ('a', 3), ('c', 4), ('b', 5)],
+        ]
+
+        for desc in records_descs:
+            items = collections.OrderedDict(desc)
+
+            query = 'SELECT ' + ', '.join(
+                ['{} as {}'.format(p[1], p[0]) for p in desc])
+
+            with self.subTest(query=query):
+                r = await self.con.fetchrow(query)
+                for idx, (field, val) in enumerate(desc):
+                    self.assertEqual(r[idx], val)
+                    self.assertEqual(r[field], items[field])
+
+                expected_repr = '<Record {}>'.format(
+                    ' '.join('{}={}'.format(p[0], p[1]) for p in desc))
+                self.assertEqual(repr(r), expected_repr)
+
+                self.assertEqual(list(r.items()), desc)
+                self.assertEqual(list(r.values()), [p[1] for p in desc])
+                self.assertEqual(list(r.keys()), [p[0] for p in desc])
 
     async def test_record_isinstance(self):
         """Test that Record works with isinstance."""