den1k / subgraph Goto Github PK

Clarifications

• make clear that subgraph is based on a fork of mapgraph
• explain the tradeoffs of decomplecting the component tree from the shape of the data - om.next

Examples

• interactive tutorial using re-frame and subgraph

Right now SubGraph stores entities directly in app-db under keys that look like [:some-keyword some-key-value]. It makes it incredibly hard to look at app-db with the naked eye or with tools like https://github.com/flexsurfer/re-frisk. I think it would make sense to use a tree structure and store the entities like {:some-keyword {some-key-value some-value}}.
So instead of e.g. this

{[:a/id 1] {:id 1 :name "x"}
 [:a/id 2] {:id 2 :name "y"}}

it would be this

{:a/id {1 {:id 1 :name "x"}
        2 {:id 2 :name "y"}}}

@julienfantin thank you for this fantastic library. I'm just getting started with it in my re-frame app. I have a certain structure for my re-frame app_db and would like to stick a subgraph db under the :subgraph key like so:

(def default-db
  {
    :ui { ... }
    :subgraph (-> (subgraph/new-db)
                  (subgraph/add-id-attr
                    :user/name
                    :color/hex))
    :server { ... }
  }
)

Is this possible? I managed writing to it with this event handler for :add using re-frame's path interceptor:

(re-frame/reg-event-db
 :add-subgraph
 [(re-frame/path :subgraph)]
 (fn [db-sg [_ & entities]]
   (apply subgraph/add db-sg entities)))

However I'm having trouble with my subscriptions, and I can't pinpoint the cause. Here is my generic query subscription:

(re-frame/reg-sub
 :sg-query
 (fn [db [pattern lookup-ref]]
   (subgraph.reframe/pull (db :subgraph) pattern lookup-ref)))

Do you think this should work?

Thank you for your help.

• make clear that subgraph is based on a fork of mapgraph
• explain the tradeoffs of decomplecting the component tree from the shape of the data - om.next

1. Should we provide a registration function that registers the pull handler?
   Yes: nobody likes setup
   No: we'd need to answer the remaining questions

2. If yes, should we define the sub id?
   Yes: it's good to standardize usage
   No: we'd need to answer the remaining question

3. If yes, should the sub id be fully-qualified?
   Yes: everything should be, and it forces a require which means we could register as a top-level
   No: Right now it'd be :vimsical.subgraph.re-frame/pull which I can't think of a good alias for

We're often faced with the unknown of wether a ref is a ref or whether we need to convert it. We've been using helper functions convert whatever is ref-convertible before calling pull. To avoid converting to refs over and over again it would be worthwhile to be able to pass a to-ref function to pull's context (today db-get-ref could be used as a hack).

https://github.com/vimsical/subgraph/blob/master/src/vimsical/subgraph.cljc#L408

Add a pull-many function to pull a seq of refs.

http://docs.datomic.com/pull.html#api

Removing entities can be achieved with (dissoc db (ref-to db my-entity)).

This is undocumented and it is a subtle operation, because:

• It will leave dangling refs in entities who join my-entity
• During query parsing pull ignores refs and colls of refs that do not resolve
• Essentially using pull gives the expected result, but the db is an inconsistent state.

If db-level consistency is desirable, we could maintain an index during add and remove that'd allow a fast reverse lookup on the removed ref, to the refs and attributes pointing to it:

{<ingress-ref> {<egress-ref> <from-attr>}}

This would give us a add and remove operations in time linear to the number of joins.

We need to document links and recursive queries syntax in that docstring

The pull api was extend by adding an options map used to parametrize database accessors in the query parser. While this allowed pull to work with ratoms as well as maps, it is not idiomatic and caused our implementation to diverge from MapGraph's.

We could simplify the pull api and bring it closer to its original implementation by defining a pull protocol:

(defprotocol Pull
  (ref? [db x])
  (get-ref [db ref]))

Pros

• Would simplify the implementation of pull
• Would make it easier to integrate upstream
• Would make the api more extensible (though extending the parser is still a non-goal)

Cons

• Would need to ship 6 implementations: hash and array maps for clj and cljs, plus ratom and atom
• Last time I checked cljs protocols incurred a non-negligible performance overhead compared to simple functions. We should definitely run some cljs benchmarks before and after these changes.

Notes

• Quick comparison of fn vs protocol performance in cljs

(def iterations 1e5)

(defn inc-in-map [x {:keys [inc-fn]}] (inc-fn x))

(let [options {:inc-fn inc}]
  (time (dotimes [_ iterations] (inc-in-map 1 options))))

;;  Compare extend-protocol vs. extend-type

(defprotocol IInc
  (inc-extend-protocol [_])
  (inc-extend-type [_]))

(extend-protocol IInc
  js/Number
  (inc-extend-protocol [x] (inc x)))

(extend-type js/Number
  IInc
  (inc-extend-type [x] (inc x)))


(time
 (dotimes [_ iterations]
   (inc-extend-protocol 1)))

(time
 (dotimes [_ iterations]
   (inc-extend-type 1)))

;; Running on node with optimizations :none
;; "Elapsed time: 16.626681 msecs"
;; "Elapsed time: 8.267159 msecs"
;; "Elapsed time: 9.751357 msecs"

;; Running on node with optimizations :advanced
;; "Elapsed time: 27.000000 msecs"
;; "Elapsed time: 8.000000 msecs"
;; "Elapsed time: 8.000000 msecs"

The parser implementation for re-frame currently uses reagent's make-reaction thus bypassing the re-frame subscription cache.

We could and probably should leverage the cache by automatically registering our subscription handler and instead use re-frame/subscribe in the parser's implementation.

We also currently don't have benchmarks for the re-frame integration and should write those first in order to compare the performance.