Upsert of datom with composite tuple containing lookup ref fails

[telekid]

I believe this is related to #378, to possibly not the exact same issue.

In the following example, I would expect the second transaction to resolve the second entity to an existing datom via the :a+b tuple (resulting in an upsert, though really a noop in this case.)

(deftest test-upsert
  (let [conn (d/create-conn {:x-ref {:db/unique :db.unique/identity}
                             :a {:db/valueType :db.type/ref}
                             :a+b {:db/valueType :db.type/tuple
                                   :db/tupleAttrs [:a :b]
                                   :db/unique :db.unique/identity}})]

    (d/transact!
     conn
     [{:db/id "x"
       :x-ref 1}
      {:a "x"
       :b 2}])

    (is (= #{[1 :x-ref 1]
             [2 :a 1]
             [2 :a+b [1 2]]
             [2 :b 2]}
           (tdc/all-datoms (d/db conn))))

    (d/transact!
     conn
     [{:db/id "x"
       :x-ref 1}
      {:a "x"
       :b 2
       :foo :bar}]) ;; upsert 2, associating :bar to :foo

    (is (= #{[1 :x-ref 1]
             [2 :a 1]
             [2 :a+b [1 2]]
             [2 :b 2]}
             [2 :foo :bar]

           (tdc/all-datoms (d/db conn))))))

However, it fails with the following exception:

1. Unhandled clojure.lang.ExceptionInfo
   Cannot add #datascript/Datom [3 :a+b [1 2] 536870914 true] because of unique
   constraint: (#datascript/Datom [2 :a+b [1 2] 536870913 true])
   {:error :transact/unique,
    :attribute :a+b,
    :datom #datascript/Datom [3 :a+b [1 2] 536870914 true]}
...

Are my expectations wrong here?

[tonsky]

Looks to me like it should work, yes

[telekid]

Awesome. I haven't poked my head into DataScript's internals before, but I'll do a bit of digging and see if I can figure out what's going on.

[telekid]

Finally got around to looking into this.

I believe what is happening is that resolve-upserts isn't accounting for the possibility of upserting via tuple containing a tempid; this causes (some? upserted-eid) to return nil, resulting in a new eid being generated here. The presence of this new entity is what causes the unique constraint violation to fail.

My guess is that you may want to move the retry-with-tempid logic into resolve-upserts, and then update that function to handle tuples + tempids.

Does that seem right?

[telekid]

After a night's rest, I came up with a different approach. Rather than using tempids as a method of resolving existing entities, leverage lookup refs. This is more idiomatic.

(deftest upsert'
  (let [conn (d/create-conn {:x-ref {:db/unique :db.unique/identity}
                             :a {:db/valueType :db.type/ref
                                 :db/unique :db.unique/identity}
                             :a+b {:db/valueType :db.type/tuple
                                   :db/tupleAttrs [:a :b]
                                   :db/unique :db.unique/identity}})]

    (d/transact!
     conn
     [{:x-ref 1}
      {:a [:x-ref 1]
       :b 2}])

    (is (= #{[1 :x-ref 1]
             [2 :a 1]
             [2 :b 2]
             [2 :a+b [1 2]]}
           (tdc/all-datoms (d/db conn))))

    (d/transact!
     conn
     [{:a [:x-ref 1]
       :b 2
       :foo :bar}])

    (is (= #{[1 :x-ref 1]
             [2 :a 1]
             [2 :a+b [1 2]]
             [2 :b 2]
             [2 :foo :bar]}

           (tdc/all-datoms (d/db conn))))))

[tonsky]

Oh! I didn’t even noticed that :a was a ref. Yeah lookup refs are probably better here