Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactable entities #389

Open
den1k opened this issue Apr 1, 2021 · 11 comments
Open

Transactable entities #389

den1k opened this issue Apr 1, 2021 · 11 comments

Comments

@den1k
Copy link

den1k commented Apr 1, 2021

This is a backwards compatible change that would vastly improve DX in my opinion

I've always wondered: why are entities not transactable? I find myself converting entities to maps all the time solely to transact them. This still causes problems when entities nest other entities. So here are a few simple ideas on how entities could be treated in transactions:

1. Entities could be treated as refs in transactions

(def schema
  {:user/friends #:db{:valueType   :db.type/ref
                      :cardinality :db.cardinality/many}})

(def ent (d/touch (d/entity @conn 1)))

ent ; eval
;; =>
{:db/id 1
 :user/email "[email protected]"
 :user/friends #{{:db/id 2} {:db/id 3}}} ; <-- nested entities

Now I convert it to a map

(def ent-map (into {:db/id (:db/id ent)} ent))
ent-map ; eval
;; =>
{:db/id 1
 :user/email "[email protected]"
 :user/friends #{{:db/id 2} {:db/id 3}}}
;; looks the same but nested entities (under :user/friends) have not been converted

I try to transact it

(d/transact! conn [(assoc ent-map :user/email "[email protected]")])
;; throws:
;; Execution error (ExceptionInfo) at datascript.db/entid (db.cljc:385).
;; Expected number or lookup ref for entity id, got #:db{:id 2}

So I can either dissoc the :user/friends map-entry or convert contained entities to refs

(d/transact! conn [(-> ent-map
                       (dissoc :genstyle.project/population)
                       ;; OR (update :user/friends #(mapv :db/id %)) 
                       (assoc :user/email "[email protected]"))])

We could spare ourselves from this by treating entities as refs in transactions. The database already walks nested data-structures to resolve refs so why not resolve entities as refs, also?

2. Entities to return maps on update

datascript.impl.entity/Entity implements clojure.lang.Associative which currently only throws errors:

clojure.lang.Associative
       ;; some methods elided
       (empty [e]         (throw (UnsupportedOperationException.)))
       (assoc [e k v]     (throw (UnsupportedOperationException.)))
       (cons  [e [k v]]   (throw (UnsupportedOperationException.)))

Instead assoc could return a hashmap

(deftype Entity [db eid touched cache]
  ;; elided
  clojure.lang.Associative
  (assoc [e k v]
    (let [e-map (cond-> {:db/id eid}
                  ; add other kvals if touched
                  touched (into e))]
     (assoc e-map k v))))

This would also make update possible. Together this means that the change of email to ent from above, could look like this:

(d/transact! conn [(assoc ent :user/email "[email protected]")])

I would've already implemented this for my own projects but unfortunately Clojure (unlike ClojureScript) doesn't allow to overwrite a Type's methods. To achieve this one would have to for DataScript and change the code of datascript.impl.entity/Entity so I wanted to raise the issue here first and see what @tonsky's thoughts are.

This would also unlock a more straightforward use of libraries like meander or specter to walk and update entities.

@tonsky
Copy link
Owner

tonsky commented Apr 1, 2021

Have you considered using pull instead? https://docs.datomic.com/on-prem/query/pull.html

@den1k
Copy link
Author

den1k commented Apr 2, 2021

Sure, I'm familiar with pull but it's slower and not dynamic in the sense that relationships can be traversed downstream.

@tonsky
Copy link
Owner

tonsky commented Apr 2, 2021

The main problem with transacting entities is exactly dynamic relationships. How far to transfer?

If you want to transact entities as refs, just (:db/id entity) and you got the same result.

As to turning entities to map, this is not exactly a great pattern. That way you’ll be transacting way more than you need, e.g. to change one attribute you’ll be transacting the whole map. This is usually not a great idea. Much better idea is to control exactly each one of the attributes you transact, not “I’ve got some map, figure out what’s different and do what you need to do”.

Datomic, as far as I understand (and I agree with it here), makes a point of separating reads and writes. Read as much as you like, transact only what’s needed.

@den1k
Copy link
Author

den1k commented Apr 3, 2021

The main problem with transacting entities is exactly dynamic relationships. How far to transfer?

I see what you mean here. For example, a deep assoc-in would result the whole chain of entities upstream to be converted into maps. Given that we can control the implementation of assoc in datascript.impl.entity/Entity maybe the returned structure can hold a change-set of datoms that can be transacted.

If you want to transact entities as refs, just (:db/id entity) and you got the same result.

True, and this has been what I've been doing but it gets hairy for nested entities.

See change-set idea above

@tonsky
Copy link
Owner

tonsky commented Apr 3, 2021

Deep assoc-in would be very confusing and error-prone

@den1k
Copy link
Author

den1k commented Apr 3, 2021

Hmm, okay the following is simpler and would work better:

  1. treat entities as refs in transactions, e.g. entity {:db/id 1} is treated as eid 1 in a transaction.
  2. assoc on entity 1 is like associng to map {:db/id 1}, so
(assoc (d/entity @conn 1) :foo "bar")
; =>
{:db/id 1 :foo "bar"} ; not transacting the whole maps with extra key values and nested relationships
  1. update returns a minimal entity-map with the looked-up key updated, e.g.
(update (d/entity @conn 1) :foo clojure.string/upper-case)
 => {:db/id 1 :foo "BAR"}
  1. assoc-in/update in also return the minimum required to make an entity transactable, e.g.
(update-in (d/entity @conn 1) [:user/friends 2] :user/email clojure.string/upper-case)
; returns hash-map 
{:db/id        1
 :user/friends [{:db/id 2}                                  ; <- Entity
                {:db/id 3 :user/email "[email protected]"} ; <- hash-map
                ]}                                          

However, the examples above break down on multiple updates, e.g.

(-> (d/touch (d/entity @conn 1)) ; {:db/id 1 :age 20}
    (update :age inc) ; {:db/id 1 :age 21}
    (update :email clojure.string/upper-case)) ; :email is missing :/

It would be possible to do this however, with a special data-structure that keeps a reference to the original Entity as well as the set of changes. This is made possible through specific implementations of assoc on this new Type

@tonsky
Copy link
Owner

tonsky commented Apr 4, 2021

Sorry, but it feels like a lot of special casing and non-standard behavior of standard clojure methods. I’ve worked with DataScript quite a bit and never felt the urge to transact entities for any reason. I trust you that you have your pains with it, but I just can’t see them (I’m trying)

@den1k
Copy link
Author

den1k commented Apr 4, 2021

All good. I might be able to implement this myself. Will keep you posted. Thank you for discussing the idea.

@den1k den1k closed this as completed Apr 4, 2021
@den1k
Copy link
Author

den1k commented Jun 16, 2021

FYI @tonsky implemented this for Datalevin. It preserves all Entity functionality and is fully backwards compatible: juji-io/datalevin#48 (comment)

What do you think?

@den1k den1k reopened this Jun 16, 2021
@tonsky
Copy link
Owner

tonsky commented Jun 17, 2021

Sound idea, but I still don’t see why this is needed? Building a proper transactions seems much more obvious and straightforward than transacting an opaque map that only transacts part of its attributes.

@den1k
Copy link
Author

den1k commented Jun 17, 2021

In my experience it eliminates a lot of code. For example, in a UI that uses Entity and passes down its ref entities through a component tree it's nice to be able to immutably assoc/dissoc/add/retract and finally transact! instead of building up verbose transaction vectors.

Especially removing an attributes or a ref under an attribute is verbose since one can't transact a map without a key but must use the vector form instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants