donut-party / dbxray

view your database's metadata skeleton and generate schemas from it 🩻
MIT License
122 stars 11 forks source link

Add circular FK support? #5

Closed harry-prins-otm closed 1 year ago

harry-prins-otm commented 1 year ago

Hi, I'm messing around with this library a bit, looks like it can be very useful!

I notice table-order (well actually, dep/depend) freaks out when it encounters a circular foreign key. Examples:

As a quick test I altered table-order to include a filter as follows:

(defn- table-order
  "used to create an omap for tables"
  [xray]
  (let [deps (table-deps xray)
        deps-set (set deps)]
    (->> deps
         (filter (fn without-circular-dependencies [[table-name dep]]
                   (let [flipped [dep table-name]]
                     (nil? (get deps-set flipped)))))
         (reduce (fn reduce-fn [g [table-name dep]]
                   (dep/depend g table-name dep))
                 (dep/graph))
         (dep/topo-sort))))

This prevents the error when calling the xray function, but I can't say for sure it still does what it's supposed to be doing correctly, or that it doesn't cause some other issue. In my case, I run into the following error when trying to call plumatic-schema on the result:

Execution error (NullPointerException) at donut.dbxray.generate.plumatic-schema/column-spec (form-init12429061481132598728.clj:41).
Cannot invoke "clojure.lang.IFn.invoke(Object, Object)" because "column_type" is null

I'm not sure this is related, though.

Oh, one minor nitpick: the name xray is used as an input variable in multiple functions, but xray is a function name as well. It may make things a bit easier on the eye if a different name is used for the inputs, or maybe the function can be named ->xray, e.g.

flyingmachine commented 1 year ago

Appreciate your writing this up! I'll try to respond soon

flyingmachine commented 1 year ago

I believe this is handled now, could you give it a try and let me know? I didn't specifically test for it, real cowboy coding over here, but if there's still a problem I'll do it the right way :P

Note that I changed the top-level structure of the xray map, it now includes a :tables key and a :table-order key. The :tables key contains what was previously the entire xray map

harry-prins-otm commented 1 year ago

This works, fantastic! Thank you very much! 🎉