cjsauer / joinery

Enables traversal of in-memory graph-like data structures using Clojure(Script)'s map protocols
Eclipse Public License 1.0
57 stars 2 forks source link

Cool project, couple of small issues #1

Open maxrothman opened 2 years ago

maxrothman commented 2 years ago

Thanks for building Joinery! I've had a lot of fun playing around with it a little recently, it definitely opens up interesting possibilities. I ran into a couple of small issues while exploring it a little, I'm happy to separate them out into separate issues if you'd prefer.

First, modifications to the joined-map don't seem to stick when you follow joins:

(def db
  {:person/id {1 {:person/name "Calvin"
                  :person/friends [[:person/id 2]]
                  :person/pet [:pet/id 1]}
               2 {:person/name "Derek"
                  :person/friends [[:person/id 1]]}}
   :pet/id    {1 {:pet/name "Malcolm"
                  :pet/species :dog
                  :pet/owner [:person/id 1]}}})

(def jm (joined-map db))
(def jm2 (assoc-in jm [:pet/id 1 :pet/breed] :collie))
(= jm2 (get-in jm2 [:person/id 1 :person/pet]))
;; => false

I suspect this is because the assoc impl modifies m but not db?

Second, calling clojure.pprint/pprint on a joined map causes infinite recursion. I think that's probably fixable by implementing the simple-dispatch multimethod? This one cropped up for me because my editor pprints REPL outputs, I suspect that's a relatively common setup.

cjsauer commented 2 years ago

Thanks for trying it out.

Indeed, the db isn't modified at all. The interface is centered around the map, m, right now in an effort to adhere to the Clojure map protocols. Further, the Joinery protocol has no notion of how to even update your db; it's focused on reads only at the moment. The default Joinery implementation is a normalized table structure, but there could exist other backing databases. As an example, say I implement Joinery for a datascript db. When one calls (assoc m :x 42), joinery can't know how to modify the datascript db in the appropriate way without additional help.

This isn't to say it can't be done, just an explanation for why it isn't yet able to do so. It would be a very cool upgrade though. It'd act almost like a zipper where you can traverse and edit in a single abstraction.

As for the infinite recursion from printing, the implementation is a bit naive, and that's likely a bug. Is it failing on the example db in the readme? Clojure or ClojureScript?

maxrothman commented 2 years ago

Indeed, the db isn't modified at all...

Makes sense! Maybe I'll poke at that at some point, seems like a fun project...

As for the infinite recursion from printing, the implementation is a bit naive, and that's likely a bug. Is it failing on the example db in the readme? Clojure or ClojureScript?

Clojure, and it happens with the example. I suspect pprint doesn't expect maps to be recursive. I think for "interesting" data structures like this one, it expects you to tell it what to do by implementing the simple-dispatch multi.