tonsky / datascript

Immutable database and Datalog query engine for Clojure, ClojureScript and JS
Eclipse Public License 1.0
5.45k stars 304 forks source link

equality on ^Entity bypasses db #433

Closed dustingetz closed 2 years ago

dustingetz commented 2 years ago

Here: https://github.com/tonsky/datascript/blob/6de343b1b3aecb95c21fbe46384face3f99b989b/src/datascript/impl/entity.cljc#L156

This breaks reactive programming semantics where, when db updates, (d/entity db x) updates but the result is = to the prior result and therefore continuous time reactive engines will skip downstream computation, reusing a buffered prior instance of the entity that is inconsistent with the latest db.

Appears to have been introduced in https://github.com/tonsky/datascript/commit/8264eb0efa500084b24654d1d2f9b69ef1d3f25f , and seems not deeply considered – will you accept a PR uncommenting that check? What needs to be tested?

tonsky commented 2 years ago

I don’t remember why we removed db check here. But comparing DBs might be expensive. How about identical? here? Would that work for you?

ggeoffrey commented 2 years ago

Provided fix accounts for db in entity equality, using identical?.

The following test is interesting. It shows this is not a perfect solution, but the tradeoff is acceptable for us.

(deftest test-entity-equality
  (let [db1 (-> (d/empty-db {})
              (d/db-with [{:db/id 1, :name "Ivan"}]))
        db2 (d/db-with db1 [])]
    (testing "Two entities are equal if they have the same :db/id and refer to the same database"
      (is (= db1 db2))
      (is (not= (d/entity db1 1) (d/entity db2 1))))))
tonsky commented 2 years ago

Thanks! Pushed 1.3.14