Skip to content

Added support for tuples values on query :in or :where#38

Closed
Quezion wants to merge 3 commits intompdairy:masterfrom
Quezion:tuple-value-support
Closed

Added support for tuples values on query :in or :where#38
Quezion wants to merge 3 commits intompdairy:masterfrom
Quezion:tuple-value-support

Conversation

@Quezion
Copy link
Copy Markdown
Contributor

@Quezion Quezion commented Oct 18, 2019

  • Updated q-analyze/resolve-any-idents to ID lookup-refs :in query. Assumed that lookup-refs are either eid in :where clause, or if in value place, that schema has :db/valueType :db.type/ref
  • Simple tests on lookup refs & tuple queries in datascript_test.cljc
  • Updated dependencies in project.clj

My understanding of the rules for differentiating lookup-refs:

;; ---- :where ----
;; All vectors in :where clause are values because lookup-refs are not allowed in them

;; ---- :in ----
;; Must look at query & understand how variable is being used in :where tuple
;; If variable used as 1st element (entity), always must be lookup-ref
;; If variable used as 3rd element (value), check attribute in schema for :db.valueType :db.type/ref
;; EX: :where [?e :foo ?v]
         :in ?v    = [:x 10]
;; -- check schema for :db/type of :foo, if not db.type/ref then it's a vector value

Fixes #37 , re-posh denistakeda/re-posh#32

This also includes #31, which I rediscovered & fixed in the course of this PR.

Dummy implementation for q-analyze/resolve-any-idents, works for
vector values but probably breaks caching for lookup refs
Assumed that lookup-refs are either eid in :where clause, or
if in value place, that schema has :db/valueType :db.type/ref
Simple tests on lookup refs & tuple queries in datascript_test.cljc
@Quezion
Copy link
Copy Markdown
Contributor Author

Quezion commented Oct 18, 2019

I definitely felt out of my depth working on this library -- but this solution satisfies the added tests on the Clojure side. I'm not sure how to run the tests on the CLJS side via cljsbuild.

There's need for a lot more tests. I could've broken something in the change to eav? -- it's a very simple "fix" & a better implementation might involve checking the :schema like resolve-any-idents does to look for :db.type/ref.

The reason I opened this PR is because I needed this changer to support my own re-frame based app. I'm going to release a sideband version of posh & re-posh off this library so I can continue with my own app development & I'll post back here with the updated dependency coordinates.

@Quezion
Copy link
Copy Markdown
Contributor Author

Quezion commented Oct 18, 2019

I've published [org.clojars.questyarbrough/posh "0.5.7"] which includes this PR. It's working for my app though it's only exercising small set of DataScript's functionality.

@Quezion
Copy link
Copy Markdown
Contributor Author

Quezion commented Oct 22, 2019

This functionality is included in pull-many PR #39, so closing this PR. Can split PRs apart & reopen on request.

@Quezion Quezion closed this Oct 22, 2019
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

Successfully merging this pull request may close these issues.

Support for tuple valueTypes

1 participant