From 1296b8090f61075bdf671350d2a0826af891dad6 Mon Sep 17 00:00:00 2001
From: Richard Newman <rnewman@twinql.com>
Date: Mon, 26 Sep 2016 14:19:41 -0700
Subject: [PATCH] Allow sets of attributes in fulltext expressions. Fixes #54.
 r=nalexander

---
 src/common/datomish/query/functions.cljc | 57 +++++++++++++++++-------
 test/datomish/test/query.cljc            | 57 ++++++++++++++++++++++++
 test/datomish/tofinoish_test.cljc        | 31 ++++++++++++-
 3 files changed, 129 insertions(+), 16 deletions(-)

diff --git a/src/common/datomish/query/functions.cljc b/src/common/datomish/query/functions.cljc
index 2117f235..3c0c27e9 100644
--- a/src/common/datomish/query/functions.cljc
+++ b/src/common/datomish/query/functions.cljc
@@ -97,12 +97,36 @@
         ;;   [(fulltext $ _ "foo") [[?x]]]
         ;;
         ;; so we instead have a placeholder attribute. Sigh.
-        attr-constant (or
-                        (and (instance? Constant attr)
-                             (not (= :any (:value attr)))
-                             (source/attribute-in-source (:source cc) (:value attr)))
-                        (and (instance? Variable attr)
-                             (cc/binding-for-symbol-or-throw cc (:symbol attr))))
+        ;; We also support sets of attributes, so you can write
+        ;;
+        ;;   [(fulltext $ #{:foo/bar :foo/baz} "Noo") [[?x]]]
+        ;;
+        ;; which involves some tomfoolery here.
+        ;;
+        ;; TODO: exclude any non-fulltext attributes. If the set shrinks to nothing,
+        ;; fail the entire pattern.
+        ;; https://github.com/mozilla/datomish/issues/56
+        attr-constants (or
+                         (and (instance? Constant attr)
+                              (let [attr (:value attr)
+                                    intern (partial source/attribute-in-source (:source cc))]
+                                (when-not (= :any attr)
+                                  (cond
+                                    (set? attr)
+                                    (map intern attr)
+
+                                    (or (keyword? attr)
+                                        (integer? attr))
+                                    (list (intern attr))
+
+                                    :else
+                                    (raise-str "Unknown fulltext attribute " attr {:attr attr})))))
+
+                         (and (instance? Variable attr)
+                              (cc/binding-for-symbol-or-throw cc (:symbol attr)))
+
+                         ;; nil, so it's seqable.
+                         nil)
 
         ;; Pull out the symbols for the binding array.
         [entity value tx score]
@@ -127,16 +151,19 @@
         wheres (concat
                  [[:match match-column match-value]      ; The FTS match.
 
-                ;; The fulltext rowid-to-datom correspondence.
-                [:=
-                 (sql/qualify datom-alias :v)
-                 (sql/qualify fulltext-alias :rowid)]]
+                  ;; The fulltext rowid-to-datom correspondence.
+                  [:=
+                   (sql/qualify datom-alias :v)
+                   (sql/qualify fulltext-alias :rowid)]]
 
-                 (when attr-constant
-                   ;; If known, the attribute itself must match.
-                   [[:=
-                     (sql/qualify datom-alias :a)
-                     attr-constant]]))
+                 ;; If known, the attribute itself must match.
+                 (when (seq attr-constants)
+                   (let [a (sql/qualify datom-alias :a)
+                         fragments (map (fn [v] [:= a v])
+                                        attr-constants)]
+                     (if (seq (rest fragments))
+                       [(cons :or fragments)]
+                       fragments))))
 
         ;; Now compose any bindings for entity, value, tx, and score.
         ;; TODO: do we need to examine existing bindings to capture
diff --git a/test/datomish/test/query.cljc b/test/datomish/test/query.cljc
index 7d239daa..def7d285 100644
--- a/test/datomish/test/query.cljc
+++ b/test/datomish/test/query.cljc
@@ -109,6 +109,26 @@
     :db/valueType :db.type/instant
     :db/cardinality :db.cardinality/many}])
 
+(def save-schema
+  [{:db/id (d/id-literal :db.part/user)
+    :db.install/_attribute :db.part/db
+    :db/cardinality :db.cardinality/one
+    :db/valueType :db.type/string
+    :db/fulltext true
+    :db/ident :save/title}
+   {:db/id (d/id-literal :db.part/user)
+    :db.install/_attribute :db.part/db
+    :db/cardinality :db.cardinality/one
+    :db/valueType :db.type/string
+    :db/fulltext true
+    :db/ident :save/excerpt}
+   {:db/id (d/id-literal :db.part/user)
+    :db.install/_attribute :db.part/db
+    :db/cardinality :db.cardinality/one
+    :db/valueType :db.type/string
+    :db/fulltext true
+    :db/ident :save/content}])
+
 (def schema-with-page
   (concat
     simple-schema
@@ -567,3 +587,40 @@
             (query/options-into-context context 10 [[:date :asc]]))
           [:order-by :limit]
           )))))
+
+(deftest-db test-parsing-fulltext conn
+  (let [attrs (<? (<initialize-with-schema conn save-schema))]
+    (is (= {:select (list [:datoms1.e :save]),
+            :modifiers [:distinct],
+            :from (list [:fulltext_values 'fulltext_values0]
+                        [:datoms 'datoms1]),
+            :where (list :and
+                         [:match :fulltext_values0.fulltext_values "something"]
+                         [:= :datoms1.v :fulltext_values0.rowid]
+                         [:= :datoms1.a (:save/title attrs)])}
+           (expand {:find '[?save]
+                    :in '[$]
+                    :where [[(list 'fulltext
+                                   '$
+                                   :save/title
+                                   "something")
+                             '[[?save]]]]}
+                  conn)))
+    (is (= {:select (list [:datoms1.e :save]),
+            :modifiers [:distinct],
+            :from (list [:fulltext_values 'fulltext_values0]
+                        [:datoms 'datoms1]),
+            :where (list :and
+                         [:match :fulltext_values0.fulltext_values "something"]
+                         [:= :datoms1.v :fulltext_values0.rowid]
+                         (list :or
+                               [:= :datoms1.a (:save/title attrs)]
+                               [:= :datoms1.a (:save/excerpt attrs)]))}
+           (expand {:find '[?save]
+                    :in '[$]
+                    :where [[(list 'fulltext
+                                   '$
+                                   #{:save/title :save/excerpt}
+                                   "something")
+                             '[[?save]]]]}
+                  conn)))))
diff --git a/test/datomish/tofinoish_test.cljc b/test/datomish/tofinoish_test.cljc
index 444f0f6b..f974fd41 100644
--- a/test/datomish/tofinoish_test.cljc
+++ b/test/datomish/tofinoish_test.cljc
@@ -127,7 +127,13 @@
     :db/cardinality :db.cardinality/one
     :db/valueType :db.type/string
     :db/fulltext true
-    :db/ident :save/content}])
+    :db/ident :save/content}
+   {:db/id (d/id-literal :db.part/user)
+    :db.install/_attribute :db.part/db
+    :db/cardinality :db.cardinality/many
+    :db/valueType :db.type/string
+    :db/fulltext false
+    :db/ident :save/unindexed}])
 
 (def tofino-schema (concat page-schema visit-schema session-schema save-schema))
 
@@ -460,3 +466,26 @@
         (is (= apricot-url "http://example.com/apricots/1"))
         (is (= apricot-title "A page about apricots."))
         (is (= apricot-excerpt "")))))))
+
+(deftest-db test-fulltext-set-attribute conn
+  (<? (d/<transact! conn tofino-schema))
+  (<? (d/<transact! conn
+                    [{:db/id             999
+                      :save/title "Whenever you want something"}
+                     {:db/id             998
+                      :save/excerpt "If there is something…"}
+                     {:db/id             997
+                      :save/unindexed "What something means…"}
+                     {:db/id             996
+                      :save/title "This is anything but."}
+                     {:db/id             995
+                      :save/content "There's something here that would match."}]))
+  (let [results
+        (<?
+          (d/<q (d/db conn)
+                {:find '[?save]
+                 :in '[$]
+                 :where [[(list 'fulltext '$ #{:save/title :save/excerpt} "something")
+                          '[[?save]]]]}))]
+    (is (= (set (map first results))
+           #{999 998}))))