threatgrid / asami

A graph store for Clojure and ClojureScript
Eclipse Public License 1.0
634 stars 29 forks source link

Read queries with edn/read; Initial work on basic interactivity #193

Closed noprompt closed 3 years ago

noprompt commented 3 years ago

Overview

The interactivity is basic and works a bit like the following.

$ asami asami:mem://db -f db.edn --interactive
?- [:find ?x :where [_ _ ?x]]
(["bob"] ,,,)
?- ^D
$ echo $?
0

I refactored the code a bit so that whether the application is executing queries from *in*, from -e <QUERY>, or interactively via --interactive, it uses the same machinery: the repl function. This is because the only difference between executing the application with the former two and the latter is the presence of a prompt.

Here is an example piping in some queries.

$ cat queries.asami | lein with-profile native run -- asami:mem://db -f db.edn
(["bob"] ,,,)
([:knows] ,,,)
([:tg/node-162] ,,,)

where the file queries.asami has the following content.

[:find ?x :where [_ _ ?x]] ; [:find ?y :where [_ ?y _]]

[:find ?z :where [?z _ _]]

The same behavior can be observed by running the application interactively.

$ lein with-profile native run -- asami:mem://db -f db.edn --interactive
?-  [:find ?x :where [_ _ ?x]] ; [:find ?y :where [_ ?y _]]
(["bob"] ,,,)
([:knows] ,,,)
?-  [:find ?z :where [?z _ _]]
([:tg/node-162] ,,,)

Some thoughts

I think we can reduce the complexity of the reader code if we accept queries as we typically do in the library e.g. [:find ?x ,,,] or {:find [?x] ,,,}. Since we're using edn/read, queries can be separated with white space (including ,) and ; can still be used for comments. This would make it easy to author queries in an .edn file (and such a file could also be accepted on the command line ie. -q queries.edn). In summary, I think we should treat input as a script written in EDN.

Also, I was confused by this line:

(let [txt (if (= \: (first (s/trim text))) (str "[" text "]") text)]
   (edn/read-string reader-opts txt))

What is this intended for?

Finally, what do you think about leveraging clojure.tools.cli for processing the command line arguments?

quoll commented 3 years ago

I don't usually write interactive CLI code, and totally spaced that it can't be interactive simply by reading for stdin. That was a big mistake. Thank you for addressing it.

On the comments:

Also, I was confused by this line:

(let [txt (if (= \: (first (s/trim text))) (str "[" text "]") text)]
(edn/read-string reader-opts txt))

What is this intended for?

When writing single line queries (especially interactively) then wrapping it in an array (or a map) seemed tedious. I figured that if a line of input looked like a query, then it should just be executed as one. So if you see a line of:

:find ?x :where [?e :prop "value"] [?e :name ?x]

Then convert it into:

[:find ?x :where [?e :prop "value"] [?e :name ?x]]

and execute it

noprompt commented 3 years ago

So if you see a line of :find ?x :where [?e :prop "value"] [?e :name ?x] Then convert it into: [:find ?x :where [?e :prop "value"] [?e :name ?x]] and execute it

Ah, okay. Now it makes sense. Is this functionality you would like the retain? At the moment, this patch does not implement that feature (you can see my confusion here) but it would be easy to support as we can rely on ; to terminate queries.

?- :find ?x
   :where [?e :prop ?x] ;

Strategically, however, I think sticking to the standard syntax of queries would provided the best foundation for growth in this part of the code base. But I won't be too pushy about it if you disagree. 😄

quoll commented 3 years ago

We can just go with requiring valid edn for now, but I would definitely like to allow for it in future. If I were actually using this interactively, then a requirement of [...] would be very annoying for me.

quoll commented 3 years ago

Forgot to address the final line:

Finally, what do you think about leveraging clojure.tools.cli for processing the command line arguments?

I did think about this. I avoided it for the following reasons:

It may be a better idea to include it. It only needs to be added to the native profile, so it's not actually an imposition on the main system. You'll note that I included cheshire for the CLI (it's the fastest JSON parser), so this library would be trivial in comparison.