Entity many ref remove and add bug

[den1k]

I'm not sure when this appeared but it is quite serious and breaks some functionality in my services as well as some existing code

running version 0.8.5

(def conn
  (d/create-conn
    nil
    {:id       {:db/valueType :db.type/string
                :db/unique    :db.unique/identity}
     :foo/refs {:db/valueType   :db.type/ref
                :db/cardinality :db.cardinality/many}}))
=> #'dash.entity.cell/conn

(d/transact! conn
             [{:id       "foo"
               :foo/refs [{:id "bar"}]}])
=> {:datoms-transacted 3}

(d/touch (d/entity @conn [:id "foo"]))
=> {:id "foo", :foo/refs #{#:db{:id 2}}, :db/id 1} ; <----------------- as expected

(d/transact! conn
             [[:db/retract [:id "foo"] :foo/refs]
              {:id       "foo"
               :foo/refs [{:id "bar"}]}])
=> {:datoms-transacted 1}

(d/touch (d/entity @conn [:id "foo"]))
=> {:id "foo", :db/id 1} ; <---------------- where is "bar"?

;; repeat same transaction:
(d/transact! conn
             [[:db/retract [:id "foo"] :foo/refs]
              {:id       "foo"
               :foo/refs [{:id "bar"}]}])
=> {:datoms-transacted 1}
(d/touch (d/entity @conn [:id "foo"]))
=> {:id "foo", :foo/refs #{#:db{:id 2}}, :db/id 1} ; <-                        "bar" reappears

;; repeating the last transaction will cause "bar" to toggle (appear & disappear from the entity)

[den1k]

Also tested with datascript and it does not have this issue

[huahaiy]

Which version did you upgrade from?

[den1k]

I’ve been almost in lockstep with releases. This is a new app that makes heavy use of replace mechanics (add & retract if the same entity annd attr in a transaction) unlike previous apps so I’m not exactly sure when this bug was introduced.

[huahaiy]

The bug was present in at least in 0.7.x.

I am still debugging it. This is what I know so far: if you split the second tx, so that the retraction and the repeat do not appear in the same transaction, it works.

The problem is that tx-data to be persisted on disk is incorrect, it should be adding a new datom, but somehow becomes deleting it, i.e. got tx=-3, instead of tx=3. The transaction logic in db.cljc is messed up somewhere. You say datascript doesn't have this problem, it is surprising, as the logic is the same.

I will continue debugging tomorrow.

[den1k]

Thank you @huahaiy. Tested with datascript versions 1.3.14 initially and just now latest (1.4.1) and confirming the bug is not present there.

[huahaiy]

Fixed. The issue is that we transact in batch, unlike datascript that transacts one datom at a time. So this bug must have been present since the first version of Datalevin.

Basically, the retracted datom can still be found in our case (since the batch is not persisted yet), and the logic will try to avoid transact it again, which leaves only the retraction remains. Now we transact always (i.e. overwrite), unless it's a unique attribute. Retracting then transacting the same thing in the same transaction is not so common, so such overwrites shouldn't have too much of a performance hit.

We will streamline these transaction logic in the future (after query engine rewrite is finished), as datalevin has different semantics from datascript.

Will try to fix the other bug, and then do a release.