Merge lp:~chipaca/u1db/fail-on-dupe-indexes-homogeneously into lp:u1db

[John Lenton]

create_index now fails with more or less the same error across all backends.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 5/20/2012 2:59 AM, John Lenton wrote:
> John Lenton has proposed merging
> lp:~chipaca/u1db/fail-on-dupe-indexes-homogeneously into lp:u1db.
>
> Requested reviews: Ubuntu One hackers (ubuntuone-hackers)
>
> For more details, see:
> https://code.launchpad.net/~chipaca/u1db/fail-on-dupe-indexes-homogeneously/+merge/106506
>
> create_index now fails with more or less the same error across all
> backends.

I'm happy to get consistency, but I believe that if the index being
created is identical to what is already there, we want it to be a
no-op and to pass. So create becomes sort of an "ensure".

In the immediate term, though, we could certainly land this for
consistency between backends.

John
=:->
 review: approve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+46EgACgkQJdeBCYSNAAPrXQCfQcqgGTusTQbpFHUu4v8P7x4X
2x8An2KvPX4cZTLj8Cp/KrTyzXvjXb8k
=doqB
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'include/u1db/u1db.h'
--- include/u1db/u1db.h	2012-05-18 15:12:42 +0000
+++ include/u1db/u1db.h	2012-05-20 00:58:21 +0000
@@ -65,6 +65,7 @@
 #define U1DB_UNHANDLED_CHARACTERS -14
 #define U1DB_MISSING_FIELD_SPECIFIER -15
 #define U1DB_INVALID_FIELD_SPECIFIER -16
+#define U1DB_DUPLICATE_INDEX_NAME -17
 #define U1DB_INTERNAL_ERROR -999
 
 // Used by put_doc_if_newer

=== modified file 'src/u1db.c'
--- src/u1db.c	2012-05-19 07:47:05 +0000
+++ src/u1db.c	2012-05-20 00:58:21 +0000
@@ -1673,7 +1673,7 @@
         "INSERT INTO index_definitions VALUES (?, ?, ?)", -1,
         &statement, NULL);
     if (status != SQLITE_OK) {
-        return status;
+        goto finish;
     }
     status = sqlite3_bind_text(statement, 1, index_name, -1, SQLITE_TRANSIENT);
     if (status != SQLITE_OK) { goto finish; }
@@ -1684,7 +1684,13 @@
                                    SQLITE_TRANSIENT);
         if (status != SQLITE_OK) { goto finish; }
         status = sqlite3_step(statement);
-        if (status != SQLITE_DONE) { goto finish; }
+        if (status != SQLITE_DONE) {
+            if (status == SQLITE_CONSTRAINT) {
+                // duplicate index definition
+                status = U1DB_DUPLICATE_INDEX_NAME;
+            }
+            goto finish;
+        }
         status = sqlite3_reset(statement);
         if (status != SQLITE_OK) { goto finish; }
     }

=== modified file 'u1db/backends/inmemory.py'
--- u1db/backends/inmemory.py	2012-05-18 15:12:42 +0000
+++ u1db/backends/inmemory.py	2012-05-20 00:58:21 +0000
@@ -171,6 +171,8 @@
         self.put_doc(doc)
 
     def create_index(self, index_name, index_expression):
+        if index_name in self._indexes:
+            raise errors.IndexNameTakenError
         index = InMemoryIndex(index_name, index_expression)
         for doc_id, (doc_rev, doc) in self._docs.iteritems():
             if doc is not None:

=== modified file 'u1db/backends/sqlite_backend.py'
--- u1db/backends/sqlite_backend.py	2012-05-18 15:12:42 +0000
+++ u1db/backends/sqlite_backend.py	2012-05-20 00:58:21 +0000
@@ -20,6 +20,7 @@
 import os
 import simplejson
 from sqlite3 import dbapi2
+import sys
 import time
 import uuid
 
@@ -693,8 +694,11 @@
             cur_fields = self._get_indexed_fields()
             definition = [(index_name, idx, field)
                           for idx, field in enumerate(index_expression)]
-            c.executemany("INSERT INTO index_definitions VALUES (?, ?, ?)",
-                          definition)
+            try:
+                c.executemany("INSERT INTO index_definitions VALUES (?, ?, ?)",
+                              definition)
+            except dbapi2.IntegrityError as e:
+                raise errors.IndexNameTakenError, e, sys.exc_info()[2]
             new_fields = set([f for f in index_expression
                               if f not in cur_fields])
             if new_fields:

=== modified file 'u1db/errors.py'
--- u1db/errors.py	2012-04-24 13:40:36 +0000
+++ u1db/errors.py	2012-05-20 00:58:21 +0000
@@ -69,6 +69,10 @@
     wire_description = "database does not exist"
 
 
+class IndexNameTakenError(U1DBError):
+    """The given index name is already taken."""
+
+
 class IndexDefinitionParseError(U1DBError):
     """The index definition cannot be parsed."""
 

=== modified file 'u1db/tests/c_backend_wrapper.pyx'
--- u1db/tests/c_backend_wrapper.pyx	2012-05-18 15:12:42 +0000
+++ u1db/tests/c_backend_wrapper.pyx	2012-05-20 00:58:21 +0000
@@ -117,6 +117,7 @@
     int U1DB_INVALID_JSON
     int U1DB_INVALID_VALUE_FOR_INDEX
     int U1DB_BROKEN_SYNC_STREAM
+    int U1DB_DUPLICATE_INDEX_NAME
     int U1DB_INTERNAL_ERROR
 
     int U1DB_INSERTED
@@ -564,6 +565,8 @@
         raise errors.BrokenSyncStream()
     if status == U1DB_CONFLICTED:
         raise errors.ConflictedDoc()
+    if status == U1DB_DUPLICATE_INDEX_NAME:
+        raise errors.IndexNameTakenError()
     raise RuntimeError('%s (status: %s)' % (context, status))
 
 

=== modified file 'u1db/tests/test_backends.py'
--- u1db/tests/test_backends.py	2012-05-19 07:47:05 +0000
+++ u1db/tests/test_backends.py	2012-05-20 00:58:21 +0000
@@ -665,6 +665,12 @@
         self.assertEqual(
             [doc], self.db.get_from_index('test-idx', [(u"valu\xe5",)]))
 
+    def test_create_index_fails_if_name_taken(self):
+        self.db.create_index('test-idx', ['key'])
+        self.assertRaises(errors.IndexNameTakenError,
+                          self.db.create_index,
+                          'test-idx', ['stuff'])
+
     def test_create_index_after_deleting_document(self):
         doc = self.db.create_doc(simple_doc)
         doc2 = self.db.create_doc(simple_doc)