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

Discrepancy between posh q and datascript q #10

Open
dpetranek opened this issue Oct 13, 2020 · 3 comments
Open

Discrepancy between posh q and datascript q #10

dpetranek opened this issue Oct 13, 2020 · 3 comments

Comments

@dpetranek
Copy link

dpetranek commented Oct 13, 2020

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
Copy link
Author

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
Copy link

dhleong commented Nov 2, 2020

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
Copy link

Folcon commented Jun 19, 2022

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?

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

3 participants