denistakeda / posh

A luxuriously simple and powerful way to make front-ends with DataScript and Reagent in Clojure.
Eclipse Public License 1.0
114 stars 9 forks source link

Discrepancy between posh `q` and datascript `q` #10

Open dpetranek opened 4 years ago

dpetranek commented 4 years ago

Steps to reproduce:

(require '[datascript.core :as ds] '[posh.reagent :as p])

(def conn (ds/create-conn))
(p/posh! conn)

(ds/transact! conn 
                 [{:app/type :type/task
                  :task/name "Create TODO"
                  :task/done? true}
                 {:app/type :type/task
                  :task/name "Learn Posh"
                  :task/done? false}
                 {:app/type :type/task
                  :task/name "Figure out queries"
                  :task/done? false}])

  @(p/q '[:find [?eid ...]
          :where
          [?eid :app/type :type/task]
          [?eid :task/done? false]]
        conn)
;; =>  [1 2 3]

  (ds/q '[:find [?eid ...]
          :where
          [?eid :app/type :type/task]
          [?eid :task/done? false]]
        @conn)
;; =>  [2 3]

I'm expecting the p/q call to return [2 3], just like the the ds/q call. I may be making a simple error, though - I'm just learning how this works.

dpetranek commented 4 years ago

I thought it might be because of a datascript version mismatch: I'm using datascript version "1.0.1", and it looks like posh is using datascript version "0.18.6".

I tried testing with the older version, though, and datascript version "0.18.6" produces a result of [3 2], which is correct. So it seems like something else.

dhleong commented 3 years ago

Possibly related: somehow the way posh processes queries is not fully compatible the way with datascript does it. For example:

(rp/reg-sub
  :contacts/query
  (fn [_ [_ query]]
    {:type :query
     :variables [(partial ds/query-rank query)]
     :query '[:find ?sort-key ?name ?email
              :in $ ?score-match
              :where
              [?e :contact/aka ?name]
              [?e :contact/emails ?email]
              [(?score-match ?name) ?sort-key]]}))

The query above works just fine directly with datascript, but in re-posh (as shown) it results in:

#error {:message "Query for unknown vars: [?sort-key]", :data {:error :parser/query, :vars #{#datascript.parser.Variable{:symbol ?sort-key}}, :form {:find [?sort-key ?name ?email], :in [[[?name ?e ?email] ...]]}}}

Possibly related, the parse-q-query fn in posh seems to just be doing what the datascript parser/query->map is doing (despite how it's commented), but also doesn't accept map-formatted queries. The posh internals are a bit opaque to me just now or I'd try to contribute something....

Folcon commented 2 years ago

I found part of the problem, but I don't know enough about the internals to know that if changing this will break anything else?

So it's a problem here, as normalize-all-eavs turns this :where:

[?e :contact/aka ?name]
[?e :contact/emails ?email]
[(?score-match ?name) ?sort-key]]

into this:

[[$ ?e :contact/aka ?name]
 [$ ?e :contact/emails ?email]
 [(?score-match ?name) ?sort-key]]

Then get-evas: Processes it like so, going from this:

[[$ ?e :contact/aka ?name]
 [$ ?e :contact/emails ?email]
 [(?score-match ?name) ?sort-key]]

into this:

([$ ?e :contact/aka ?name] [$ ?e :contact/emails ?email])

Finally, get-all-vars extracts the vars:

#{?email ?e ?name}

So I'm unclear which function is misbehaving, should get-all-vars be getting the original query and thus returning these added vars? Which it can do:

(let [where '[[?e :contact/aka ?name]
              [?e :contact/emails ?email]
              [(?score-match ?name) ?sort-key]]]
  (-> where
    (posh.lib.q-analyze/normalize-all-eavs)
    ;;(posh.lib.q-analyze/get-eavs)
    (posh.lib.q-analyze/get-all-vars)))
=> #{?email ?sort-key ?score-match ?e ?name}

@denistakeda is there an obvious thing to do here? I'd be happy to write a patch if you can point what the "correct" behaviour is =)...

To be clear, just doing this here:

vars (vec (get-all-vars where #_eavs))

Does get my code to run, so it's an answer, just not sure if it's breaking some subtle caching behaviour elsewhere...

EDIT: An update, I've done some initial transaction / update tests and everything seems to work? Still not 100% confident that it's all good, but maybe? Let me know if you're more confident that me, happy to write a quick PR. EDIT2: Ok, some further testing shows that (vec (get-all-vars query #_where #_eavs)) works better than where, which fails on cases when you're passing in a dbvar into your query via an :in.

Hmm, there also seems to be an odd behaviour with :reload-patterns when there's an or in the :where.

I've been using (select-keys @(posh.plugin-base/get-posh-atom posh.clj.datascript/dcfg conn) [:graph :ratoms :reactions :cache :filters :retrieve]) to look at what the posh conn state looks like and if there are reload-patterns, then you get a lot of empty vectors for each clause in the or, which seems odd?