From a4af151e8bff04d4afb56860969dcbd7569efb16 Mon Sep 17 00:00:00 2001 From: Nikita Prokopov Date: Fri, 3 May 2024 22:52:47 +0200 Subject: [PATCH] Regression: fixed some OR queries broken in 1.6.4 (closes #468, closes #469) --- CHANGELOG.md | 4 ++ src/datascript/query.cljc | 82 +++++++++++++++++------------- test/datascript/test/query_or.cljc | 28 ++++++++++ 3 files changed, 78 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8e3166e..598e98fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 1.6.5 - May 3, 2024 + +- Regression: fixed some OR queries broken in 1.6.4 #468 #469 + # 1.6.4 - Implement “constant substitution” optimization for queries #462 diff --git a/src/datascript/query.cljc b/src/datascript/query.cljc index b08c0953..a53a1429 100644 --- a/src/datascript/query.cljc +++ b/src/datascript/query.cljc @@ -388,13 +388,45 @@ (assoc a :tuples (filterv #(nil? (hash (key-fn-a %))) tuples-a)))) -(defn lookup-pattern-db [db pattern] +(defn- rel-with-attr [context sym] + (some #(when (contains? (:attrs %) sym) %) (:rels context))) + +(defn substitute-constant [context pattern-el] + (when (free-var? pattern-el) + (when-some [rel (rel-with-attr context pattern-el)] + (when-some [tuple (first (:tuples rel))] + (when (nil? (fnext (:tuples rel))) + (let [idx (get (:attrs rel) pattern-el)] + (#?(:cljs da/aget :clj get) tuple idx))))))) + +(defn substitute-constants [context pattern] + (mapv #(or (substitute-constant context %) %) pattern)) + +(defn resolve-pattern-lookup-refs [source pattern] + (if (satisfies? db/IDB source) + (let [[e a v tx] pattern + e' (if (or (lookup-ref? e) (attr? e)) + (db/entid-strict source e) + e) + v' (if (and v (attr? a) (db/ref? source a) (or (lookup-ref? v) (attr? v))) + (db/entid-strict source v) + v) + tx' (if (lookup-ref? tx) + (db/entid-strict source tx) + tx)] + (subvec [e' a v' tx'] 0 (count pattern))) + pattern)) + +(defn lookup-pattern-db [context db pattern] ;; TODO optimize with bound attrs min/max values here - (let [search-pattern (mapv #(if (or (= % '_) (free-var? %)) nil %) pattern) + (let [search-pattern (->> pattern + (substitute-constants context) + (resolve-pattern-lookup-refs db) + (mapv #(if (or (= % '_) (free-var? %)) nil %))) datoms (db/-search db search-pattern) attr->prop (->> (map vector pattern ["e" "a" "v" "tx"]) - (filter (fn [[s _]] (free-var? s))) - (into {}))] + (filter (fn [[s _]] (free-var? s))) + (into {}))] (Relation. attr->prop datoms))) (defn matches-pattern? [pattern tuple] @@ -408,11 +440,11 @@ false)) true))) -(defn lookup-pattern-coll [coll pattern] +(defn lookup-pattern-coll [context coll pattern] (let [data (filter #(matches-pattern? pattern %) coll) attr->idx (->> (map vector pattern (range)) - (filter (fn [[s _]] (free-var? s))) - (into {}))] + (filter (fn [[s _]] (free-var? s))) + (into {}))] (Relation. attr->idx (mapv to-array data)))) ;; FIXME to-array (defn normalize-pattern-clause [clause] @@ -420,12 +452,10 @@ clause (concat ['$] clause))) -(defn lookup-pattern [source pattern] - (cond - (satisfies? db/ISearch source) - (lookup-pattern-db source pattern) - :else - (lookup-pattern-coll source pattern))) +(defn lookup-pattern [context source pattern] + (if (satisfies? db/ISearch source) + (lookup-pattern-db context source pattern) + (lookup-pattern-coll context source pattern))) (defn collapse-rels [rels new-rel] (loop [rels rels @@ -437,9 +467,6 @@ (recur (next rels) new-rel (conj acc rel))) (conj acc new-rel)))) -(defn- rel-with-attr [context sym] - (some #(when (contains? (:attrs %) sym) %) (:rels context))) - (defn- context-resolve-val [context sym] (when-some [rel (rel-with-attr context sym)] (when-some [tuple (first (:tuples rel))] @@ -667,21 +694,6 @@ rel)))))))) rel)))) -(defn resolve-pattern-lookup-refs [source pattern] - (if (satisfies? db/IDB source) - (let [[e a v tx] pattern - e' (if (or (lookup-ref? e) (attr? e)) - (db/entid-strict source e) - e) - v' (if (and v (attr? a) (db/ref? source a) (or (lookup-ref? v) (attr? v))) - (db/entid-strict source v) - v) - tx' (if (lookup-ref? tx) - (db/entid-strict source tx) - tx)] - (subvec [e' a v' tx'] 0 (count pattern))) - pattern)) - (defn dynamic-lookup-attrs [source pattern] (let [[e a v tx] pattern] (cond-> #{} @@ -808,12 +820,10 @@ '[*] ;; pattern (let [source *implicit-source* - pattern (->> clause - (substitute-constants context) - (resolve-pattern-lookup-refs source)) - relation (lookup-pattern source pattern)] + pattern' (resolve-pattern-lookup-refs source clause) + relation (lookup-pattern context source pattern')] (binding [*lookup-attrs* (if (satisfies? db/IDB source) - (dynamic-lookup-attrs source pattern) + (dynamic-lookup-attrs source pattern') *lookup-attrs*)] (update context :rels collapse-rels relation)))))) diff --git a/test/datascript/test/query_or.cljc b/test/datascript/test/query_or.cljc index aacf3a7f..0203f620 100644 --- a/test/datascript/test/query_or.cljc +++ b/test/datascript/test/query_or.cljc @@ -190,6 +190,34 @@ ($2 or ($ or [?e :name "Ivan"]))] #{1}))) +(deftest ^{:doc "issue-468, issue-469"} test-const-substitution + (let [db (-> (d/empty-db {:parent {:db/valueType :db.type/ref}}) + (d/db-with [{:db/id "Ivan" :name "Ivan"} + {:db/id "Oleg" :name "Oleg" :parent "Ivan"} + {:db/id "Petr" :name "Petr" :parent "Oleg"}]))] + (is (= #{["Ivan" 1 2]} + (d/q '[:find ?name ?x ?y + :in $ ?name + :where + [?x :name ?name] + (or-join [?x ?y] + (and + [?x :parent ?z] + [?z :parent ?y]) + [?y :parent ?x])] + db "Ivan"))) + + (is (= #{} + (d/q '[:find ?name ?x ?y + :in $ ?name + :where + [?x :name ?name] + (or-join [?x ?y] + (and + [?x :parent ?z] + [?z :parent ?y]) + [?x :parent ?y])] + db "Ivan"))))) (deftest test-errors (is (thrown-with-msg? ExceptionInfo #"All clauses in 'or' must use same set of free vars, had \[#\{\?e\} #\{(\?a \?e|\?e \?a)\}\] in \(or \[\?e :name _\] \[\?e :age \?a\]\)"