Feature: bind queries #397
Conversation
quot is handy for integer divison & subStr lets you get substrings w/ start and end indices instead of regexen.
...async or otherwise
They weren't actually doing the logging before! I had recently converted these from fns, so didn't notice the need for this at first.
src/fluree/db/query/exec/where.cljc
Outdated
| (defn add-fn-result-to-solution | ||
| [solution var-name result] | ||
| (-> solution | ||
| (assoc var-name {::var var-name ::val result}))) |
There was a problem hiding this comment.
We might also have to infer the data type here and add it to the map in case we ever want to use one of these bound values in an index scan while matching some later pattern. Index scans will come out with weird results with nil as the data type because of the way our sorted-set comparators treat nils.
| @@ -93,3 +97,40 @@ | |||
|
|
|||
| #?(:cljs | |||
| (log-to-console!)) | |||
|
|
|||
| #?(:clj | |||
| (defmacro debug->>val | |||
There was a problem hiding this comment.
boy oh boy are these macros gonna come in handy.
src/fluree/db/query/exec/where.cljc
Outdated
| [_db solution pattern _ error-ch] | ||
| (let [bind (val pattern) | ||
| out-ch (async/chan 2) | ||
| binds-ch (async/to-chan! (vals bind))] |
There was a problem hiding this comment.
I could be wrong, but I don't think we want to put the individual binds in a channel to be evaluated individually. I think we want all the binds to apply to the solution at once instead of the making multiple copies and applying each bind individually to one of the copies. I also don't think we need the coordination that pipeline-async gives us since the user-supplied functions won't be asynchronous.
If we have n vars in a bind map, then this would output n new solutions for each input solution, each with only one of those vars bound. I think we should instead output only one new solution for each input solution with all n vars bound in it together.
:bind pattern processing still needs to be async in case the user-supplied function throws an exception so we can put that exception onto the error channel, but I think we can get away with using the go block starting at line 366 and putting a reduce over (vals bind) that computes all the binds and adds them to the solution at once, and then returns the new solution with all the binds added instead of funneling each of the binds individually through a channel into pipeline-async.
src/fluree/db/query/exec/eval.cljc
Outdated
|
|
||
| (def allowed-symbols | ||
| (set/union allowed-aggregates allowed-filters)) | ||
| (set/union allowed-aggregates allowed-fns)) |
There was a problem hiding this comment.
This is pretty minor, but we might need a better name than allowed-fns, because all of the aggregates are also functions; they just act on different kinds of inputs (grouped values vs individual values). It doesn't matter in practice, but it might make the code a little more confusing for the uninitiated. I get why "filters" is no longer a good name because it's to specific. Those functions now are used in more than just filters, but I think "fns" is too general. I can't think of a better name right now, but I'll noodle over it.
There was a problem hiding this comment.
What do you think of my proposal in e8e2945?
test/fluree/db/query/fql_test.clj
Outdated
| :where [[?s :schema/age ?age] | ||
| {:bind {?decadesOld (quot ?age 10)}} | ||
| [?s :schema/name ?name] | ||
| {:bind {?firstLetterOfName (subStr ?name 0 1)}}] |
There was a problem hiding this comment.
I think it might be useful to have a test that has a bind map with multiple entries. Maybe even combine the two here (after the pattern involving ?name).
...to hopefully better reflect their intent
...instead of one-by-one
...on a channel
...so it can be used in binds too.
...until we actually implement it.
|
OK @zonotope I think this is ready for re-review |
| @@ -16,13 +16,13 @@ | |||
| [?s :ex/favNums ?favNums]] | |||
| :group-by ?name} | |||
| subject @(fluree/query db qry)] | |||
| (is (= [["Cam" 2] ["Alice" 3] ["Brian" 1]] | |||
| (is (= [["Liam" 2] ["Cam" 2] ["Alice" 3] ["Brian" 1]] | |||
There was a problem hiding this comment.
this name looks familiar :)
Closes #314
Adds support for bind maps (only; it does implement the old bind tuple format) that only work on individual results, not aggregates across all results (to better align with the SPARQL spec).
Also adds a couple of useful query fns:
quotandsubStrfor integer division and indexed substring support, respectively.I left in a couple of commented-out usages of
log/debug-async->valsandlog/debug-async->>valsto demonstrate how they're used. But there are probably runtime perf considerations with how those work even if you're log level is higher thanDEBUG, hence the commenting-out.