Skip to content

Commit

Permalink
Regression: fixed some OR queries broken in 1.6.4 (closes #468, closes …
Browse files Browse the repository at this point in the history 
…#469)
  • Loading branch information
@tonsky
tonsky committed May 3, 2024
1 parent 8bbbc25 commit a4af151
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 36 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
82 changes: 46 additions & 36 deletions src/datascript/query.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -408,24 +440,22 @@
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]
(if (source? (first clause))
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
Expand All @@ -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))]
Expand Down Expand Up @@ -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-> #{}
Expand Down Expand Up @@ -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))))))

Expand Down
28 changes: 28 additions & 0 deletions test/datascript/test/query_or.cljc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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\]\)"
Expand Down

0 comments on commit a4af151

Please sign in to comment.