Fixed upsert incorectly failing tempid check (closes #363)

Author: tonsky

CHANGELOG.md

@@ -1,3 +1,7 @@
+# WIP
+
+- Fixed upsert incorectly failing tempid check #363
+
 # 1.0.0
 
 - Support composite tuples #323

src/datascript/db.cljc

@@ -438,6 +438,9 @@
 (defprotocol ISearch
   (-search [data pattern]))
 
+(defn- ^Datom fsearch [data pattern]
+  (first (-search data pattern)))
+
 (defprotocol IIndexAccess
   (-datoms [db index components])
   (-seek-datoms [db index components])
@@ -1018,7 +1021,7 @@
         indexing? (update :avet set/conj datom cmp-datoms-avet-quick)
         true      (advance-max-eid (.-e datom))
         true      (assoc :hash (atom 0)))
-      (if-some [removing (first (-search db [(.-e datom) (.-a datom) (.-v datom)]))]
+      (if-some [removing (fsearch db [(.-e datom) (.-a datom) (.-v datom)])]
         (cond-> db
           true      (update :eavt set/disj removing cmp-datoms-eavt-quick)
           true      (update :aevt set/disj removing cmp-datoms-aevt-quick)
@@ -1204,18 +1207,22 @@
         db        (:db-after report)
         e         (entid-strict db e)
         v         (if (ref? db a) (entid-strict db v) v)
-        new-datom (datom e a v tx)]
-    (if (multival? db a)
-      (if (empty? (-search db [e a v]))
+        new-datom (datom e a v tx)
+        multival? (multival? db a)
+        old-datom ^Datom (if multival?
+                           (fsearch db [e a v])
+                           (fsearch db [e a]))]
+      (cond
+        (nil? old-datom)
         (transact-report report new-datom)
-        report)
-      (if-some [^Datom old-datom (first (-search db [e a]))]
-        (if (= (.-v old-datom) v)
-          report
-          (-> report
-            (transact-report (datom e a (.-v old-datom) tx false))
-            (transact-report new-datom)))
-        (transact-report report new-datom)))))
+
+        (= (.-v old-datom) v)
+        (update report ::tx-redundant conjv new-datom)
+
+        :else
+        (-> report
+          (transact-report (datom e a (.-v old-datom) tx false))
+          (transact-report new-datom)))))
 
 (defn- transact-retract-datom [report ^Datom d]
   (let [tx (current-tx report)]
@@ -1232,7 +1239,7 @@
   (if (contains? (:tempids initial-report) tempid)
     (raise "Conflicting upsert: " tempid " resolves"
            " both to " upserted-eid " and " (get-in initial-report [:tempids tempid])
-      { :error :transact/upsert })
+      {:error :transact/upsert})
     ;; try to re-run from the beginning
     ;; but remembering that `tempid` will resolve to `upserted-eid`
     (let [tempids' (-> (:tempids report)
@@ -1275,9 +1282,9 @@
                       (if (datom-added datom)
                         (dissoc tempids (:e datom))
                         tempids))
-        unused      (reduce reduce-fn all-tempids (:tx-data report))]
+        unused      (reduce reduce-fn all-tempids (concat (:tx-data report) (::tx-redundant report)))]
     (if (empty? unused)
-      (dissoc report ::value-tempids)
+      (dissoc report ::value-tempids ::tx-redundant)
       (raise "Tempids used only as value in transaction: " (sort (vals unused))
         {:error :transact/syntax, :tempids unused}))))
 
@@ -1377,7 +1384,7 @@
             (and (keyword? op)
               (not (builtin-fn? op)))
             (if-some [ident (entid db op)]
-              (let [fun  (-> (-search db [ident :db/fn]) first :v)
+              (let [fun  (:v (fsearch db [ident :db/fn]))
                     args (next entity)]
                 (if (fn? fun)
                   (recur report (concat (apply fun db args) entities))
@@ -1441,7 +1448,6 @@
             (raise "Can’t modify tuple attrs directly: " entity
               {:error :transact/syntax, :tx-data entity})
 
-
             (= op :db/add)
             (recur (transact-add report entity) entities)
 
@@ -1450,7 +1456,7 @@
               (let [v (if (ref? db a) (entid-strict db v) v)]
                 (validate-attr a entity)
                 (validate-val v entity)
-                (if-some [old-datom (first (-search db [e a v]))]
+                (if-some [old-datom (fsearch db [e a v])]
                   (recur (transact-retract-datom report old-datom) entities)
                   (recur report entities)))
               (recur report entities))

test/datascript/test/transact.cljc

@@ -249,21 +249,45 @@
       (is (:had-birthday e)))))
 
 (deftest test-resolve-eid
-  (let [conn (d/create-conn)
-        t1   (d/transact! conn [[:db/add -1 :name "Ivan"]
-                                [:db/add -1 :age 19]
-                                [:db/add -2 :name "Petr"]
-                                [:db/add -2 :age 22]])
-        t2   (d/transact! conn [[:db/add "Serg" :name "Sergey"]
-                                [:db/add "Serg" :age 30]])]
-    (is (= (:tempids t1) { -1 1, -2 2, :db/current-tx (+ d/tx0 1) }))
-    (is (= (:tempids t2) { "Serg" 3, :db/current-tx (+ d/tx0 2) }))
-    (is (= #{[1 "Ivan" 19   (+ d/tx0 1)]
-             [2 "Petr" 22   (+ d/tx0 1)]
-             [3 "Sergey" 30 (+ d/tx0 2)]}
-           (d/q '[:find  ?e ?n ?a ?t
-                  :where [?e :name ?n ?t]
-                         [?e :age ?a]] @conn)))))
+  (let [db (d/empty-db {:name {:db/unique :db.unique/identity}
+                        :aka  {:db/unique :db.unique/identity
+                               :db/cardinality :db.cardinality/many}
+                        :ref  {:db/valueType :db.type/ref}})]
+    (let [report (d/with db [[:db/add -1 :name "Ivan"]
+                             [:db/add -1 :age 19]
+                             [:db/add -2 :name "Petr"]
+                             [:db/add -2 :age 22]
+                             [:db/add "Serg" :name "Sergey"]
+                             [:db/add "Serg" :age 30]])]
+      (is (= (:tempids report)
+            {-1 1
+             -2 2
+             "Serg" 3
+             :db/current-tx (+ d/tx0 1) }))
+      (is (= #{[1 :name "Ivan"]
+               [1 :age 19]
+               [2 :name "Petr"]
+               [2 :age 22]
+               [3 :name "Sergey"]
+               [3 :age 30]}
+            (tdc/all-datoms (:db-after report)))))
+
+    (let [db' (d/db-with db [[:db/add -1 :name "Ivan"]
+                             [:db/add -2 :ref -1]])]
+      (is (= #{[1 :name "Ivan"] [2 :ref 1]}
+            (tdc/all-datoms db'))))
+
+    (testing "#363"
+      (let [db' (-> db
+                  (d/db-with [[:db/add -1 :name "Ivan"]])
+                  (d/db-with [[:db/add -1 :name "Ivan"]
+                              [:db/add -2 :ref -1]]))]
+        (is (= #{[1 :name "Ivan"] [2 :ref 1]} (tdc/all-datoms db'))))
+      (let [db' (-> db
+                  (d/db-with [[:db/add -1 :aka "Batman"]])
+                  (d/db-with [[:db/add -1 :aka "Batman"]
+                              [:db/add -2 :ref -1]]))]
+        (is (= #{[1 :aka "Batman"] [2 :ref 1]} (tdc/all-datoms db')))))))
 
 (deftest test-tempid-ref-295
   (let [db (-> (d/empty-db {:ref {:db/unique :db.unique/identity