Add support to specify query binding arguments as symbols

[k13gomez]

This pull request adds support to specify query binding arguments as symbols instead of only keywords so that defquery syntax looks closer to function definition syntax.

Example from test case added:

(defquery temperature-below-value-using-symbol-arg
  [?value]
  [:temperature [{value :value}] (< value ?value)])

[EthanEChristian]

Overall i don't see any issues with updating the query contract like this, but perhaps @mrrodriguez or @WilliamParker might.

I do however think it would be nice to update the doc to include the expectation that the arg of a query is either a keyword or symbol.

[k13gomez]

I can open a separate PR to update the doc as well.

[EthanEChristian]

I will try and keep a look out for the PR to clara-site

[EthanEChristian]

Oops just noticed the test failures

[k13gomez]

Yes, I did too, I'm working on fixing them, will let you know when it's good to go.

[WilliamParker]

This test fails:

(deftest test-defquery
  (let [session (-> (mk-session [cold-query])
                    (insert (->Temperature 15 "MCI"))
                    (insert (->Temperature 20 "MCI")) ; Test multiple items in result.
                    (insert (->Temperature 10 "ORD"))
                    (insert (->Temperature 35 "BOS"))
                    (insert (->Temperature 80 "BOS"))
                    fire-rules)]


    ;; Query by location.
    (is (= #{{:?l "BOS" :?t 35}}
           (set (query session cold-query '?l "BOS"))))

    (is (= #{{:?l "MCI" :?t 15} {:?l "MCI" :?t 20}}
           (set (query session cold-query '?l "MCI"))))

    (is (= #{{:?l "ORD" :?t 10}}
           (set (query session cold-query '?l "ORD"))))))

That is, in the query operation params are still expected to be keywords. My snap response right now is that this divergence is probably fine but will give it a bit more thought.

[k13gomez]

@WilliamParker @EthanEChristian I think I've addressed all the feedback, tests are passing, and all changes have been squashed together.

[mrrodriguez]

This test fails:

(deftest test-defquery
  (let [session (-> (mk-session [cold-query])
                    (insert (->Temperature 15 "MCI"))
                    (insert (->Temperature 20 "MCI")) ; Test multiple items in result.
                    (insert (->Temperature 10 "ORD"))
                    (insert (->Temperature 35 "BOS"))
                    (insert (->Temperature 80 "BOS"))
                    fire-rules)]


    ;; Query by location.
    (is (= #{{:?l "BOS" :?t 35}}
           (set (query session cold-query '?l "BOS"))))

    (is (= #{{:?l "MCI" :?t 15} {:?l "MCI" :?t 20}}
           (set (query session cold-query '?l "MCI"))))

    (is (= #{{:?l "ORD" :?t 10}}
           (set (query session cold-query '?l "ORD"))))))

That is, in the query operation params are still expected to be keywords. My snap response right now is that this divergence is probably fine but will give it a bit more thought.

I think I'd rather see the output always be keywords. It is a data structure at that point and we should use consistent types there. In general, it'd pretty odd to use symbol-keyed maps in clj anyways, so it'd also be fairly non-idiomatic.

The symbols used in the actual definition of the query are more of a syntactic convenience to look closer to how one may expect things to be from a DSL perspective. I don't think the output needs to directly mirror the syntax used as input.

[mrrodriguez]

#463 (comment) I believe this is how it is implemented (always returning kw keys for output)

So this all looks fine to me now.

[EthanEChristian]

@WilliamParker
Unless you have additional comments, i am going to try and get this merged pretty soon.

[k13gomez]

@EthanEChristian I've opened the docs update, please let me know if you have any suggestions: oracle-samples/clara-site#38

[WilliamParker]

I missed this, but basically this and the docs change look fine, +1.