From a1cc372d43d501e367699ee8c240dbbef509d436 Mon Sep 17 00:00:00 2001
From: Nick Alexander <nalexander@mozilla.com>
Date: Sun, 7 Aug 2016 22:02:51 -0700
Subject: [PATCH] Fix :db.unique/value, which should be per (a, v) pair, not
 per v-value.

---
 src/datomish/db.cljc            |  5 ++---
 src/datomish/sqlite_schema.cljc | 23 +++++++++--------------
 test/datomish/db_test.cljc      | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/src/datomish/db.cljc b/src/datomish/db.cljc
index 634ec603..8762c448 100644
--- a/src/datomish/db.cljc
+++ b/src/datomish/db.cljc
@@ -264,13 +264,12 @@
                         (<? (<insert-fulltext-value db v))
                         v)]
                 (<? (exec
-                      ["INSERT INTO datoms VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)" e a v tx
+                      ["INSERT INTO datoms VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)" e a v tx
                        tag ;; value_type_tag
                        (ds/indexing? schema a) ;; index_avet
                        (ds/ref? schema a) ;; index_vaet
                        fulltext? ;; index_fulltext
-                       (ds/unique-value? schema a) ;; unique_value
-                       (ds/unique-identity? schema a) ;; unique_identity
+                       (ds/unique? schema a) ;; unique_value
                        ])))
               (if fulltext?
                 (<? (exec
diff --git a/src/datomish/sqlite_schema.cljc b/src/datomish/sqlite_schema.cljc
index 768b96b9..f548e50b 100644
--- a/src/datomish/sqlite_schema.cljc
+++ b/src/datomish/sqlite_schema.cljc
@@ -22,7 +22,7 @@
                          value_type_tag SMALLINT NOT NULL,
                          index_avet TINYINT NOT NULL DEFAULT 0, index_vaet TINYINT NOT NULL DEFAULT 0,
                          index_fulltext TINYINT NOT NULL DEFAULT 0,
-                         unique_value TINYINT NOT NULL DEFAULT 0, unique_identity TINYINT NOT NULL DEFAULT 0)"
+                         unique_value TINYINT NOT NULL DEFAULT 0)"
    "CREATE INDEX idx_datoms_eavt ON datoms (e, a, value_type_tag, v)"
    "CREATE INDEX idx_datoms_aevt ON datoms (a, e, value_type_tag, v)"
 
@@ -38,16 +38,11 @@
    ;; exclusive.
    "CREATE INDEX idx_datoms_fulltext ON datoms (value_type_tag, v, a, e) WHERE index_fulltext IS NOT 0"
 
-   ;; TODO: possibly remove this index.  :db.unique/value should be asserted by the transactor in
-   ;; all cases, but the index may speed up some of SQLite's query planning.  For now, it services
-   ;; to validate the transactor implementation.  Note that tag is needed here, since we could have
-   ;; a keyword (stored as ":foo") that overlaps a string value ":foo".
-   "CREATE UNIQUE INDEX idx_datoms_unique_value ON datoms (value_type_tag, v) WHERE unique_value IS NOT 0"
-   ;; TODO: possibly remove this index.  :db.unique/identity should be asserted by the transactor in
-   ;; all cases, but the index may speed up some of SQLite's query planning.  For now, it serves to
-   ;; validate the transactor implementation.  Note that tag is needed here to differentiate, e.g.,
-   ;; keywords and strings.
-   "CREATE UNIQUE INDEX idx_datoms_unique_identity ON datoms (a, value_type_tag, v) WHERE unique_identity IS NOT 0"
+   ;; TODO: possibly remove this index.  :db.unique/{value,identity} should be asserted by the
+   ;; transactor in all cases, but the index may speed up some of SQLite's query planning.  For now,
+   ;; it serves to validate the transactor implementation.  Note that tag is needed here to
+   ;; differentiate, e.g., keywords and strings.
+   "CREATE UNIQUE INDEX idx_datoms_unique_value ON datoms (a, value_type_tag, v) WHERE unique_value IS NOT 0"
 
    "CREATE TABLE transactions (e INTEGER NOT NULL, a SMALLINT NOT NULL, v BLOB NOT NULL, tx INTEGER NOT NULL, added TINYINT NOT NULL DEFAULT 1, value_type_tag SMALLINT NOT NULL)"
    "CREATE INDEX idx_transactions_tx ON transactions (tx)"
@@ -65,17 +60,17 @@
 
    ;; A view transparently interpolating fulltext indexed values into the datom structure.
    "CREATE VIEW fulltext_datoms AS
-     SELECT e, a, fulltext_values.text AS v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value, unique_identity
+     SELECT e, a, fulltext_values.text AS v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value
        FROM datoms, fulltext_values
        WHERE datoms.index_fulltext IS NOT 0 AND datoms.v = fulltext_values.rowid"
 
    ;; A view transparently interpolating all entities (fulltext and non-fulltext) into the datom structure.
    "CREATE VIEW all_datoms AS
-     SELECT e, a, v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value, unique_identity
+     SELECT e, a, v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value
        FROM datoms
        WHERE index_fulltext IS 0
      UNION ALL
-     SELECT e, a, v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value, unique_identity
+     SELECT e, a, v, tx, value_type_tag, index_avet, index_vaet, index_fulltext, unique_value
        FROM fulltext_datoms"
 
    ;; Materialized views of the schema.
diff --git a/test/datomish/db_test.cljc b/test/datomish/db_test.cljc
index 29b8243a..3c5d4a81 100644
--- a/test/datomish/db_test.cljc
+++ b/test/datomish/db_test.cljc
@@ -657,3 +657,35 @@
 
               (finally
                 (<? (d/<close conn))))))))))
+
+(deftest-async test-unique-value
+  (with-tempfile [t (tempfile)]
+    (let [conn (<? (d/<connect t))]
+      (try
+        (let [tx0 (:tx (<? (d/<transact! conn [{:db/id                 (d/id-literal :db.part/user -1)
+                                                :db/ident              :test/x
+                                                :db/unique             :db.unique/value
+                                                :db/valueType          :db.type/long
+                                                :db.install/_attribute :db.part/db}
+                                               {:db/id                 (d/id-literal :db.part/user -2)
+                                                :db/ident              :test/y
+                                                :db/unique             :db.unique/value
+                                                :db/valueType          :db.type/long
+                                                :db.install/_attribute :db.part/db}])))]
+
+          (testing "can insert different :db.unique/value attributes with the same value"
+            (let [report1 (<? (d/<transact! conn [[:db/add (d/id-literal :db.part/user -1) :test/x 12345]]))
+                  eid1    (get-in report1 [:tempids (d/id-literal :db.part/user -1)])
+                  report2 (<? (d/<transact! conn [[:db/add (d/id-literal :db.part/user -2) :test/y 12345]]))
+                  eid2    (get-in report2 [:tempids (d/id-literal :db.part/user -2)])]
+              (is (= (<? (<datoms-after (d/db conn) tx0))
+                     #{[eid1 :test/x 12345]
+                       [eid2 :test/y 12345]}))))
+
+          (testing "can't upsert a :db.unique/value field"
+            (is (thrown-with-msg?
+                  ExceptionInfo #"because of unique constraint"
+                  (<? (d/<transact! conn [{:db/id (d/id-literal :db.part/user -1) :test/x 12345 :test/y 99999}]))))))
+
+        (finally
+          (<? (d/<close conn)))))))