Fix bug in same-keys? / sum-rel

Author: galdre

src/datascript/query.cljc

@@ -61,7 +61,7 @@
 (defn same-keys? [a b]
   (and (= (count a) (count b))
        (every? #(contains? b %) (keys a))
-       (every? #(contains? b %) (keys a))))
+       (every? #(contains? a %) (keys b))))
 
 (defn- looks-like? [pattern form]
   (cond
@@ -126,38 +126,43 @@
 
 #?(:clj (set! *unchecked-math* false))
 
+(defn- sum-rel* [attrs-a tuples-a attrs-b tuples-b]
+  (let [idxb->idxa (vec (for [[sym idx-b] attrs-b]
+                          [idx-b (attrs-a sym)]))
+        tlen    (->> (vals attrs-a) (reduce max) (inc))
+        tuples' (persistent!
+                 (reduce
+                  (fn [acc tuple-b]
+                    (let [tuple' (da/make-array tlen)]
+                      (doseq [[idx-b idx-a] idxb->idxa]
+                        (aset tuple' idx-a (#?(:cljs da/aget :clj get) tuple-b idx-b)))
+                      (conj! acc tuple')))
+                  (transient (vec tuples-a))
+                  tuples-b))]
+    (Relation. attrs-a tuples')))
+
 (defn sum-rel [a b]
   (let [{attrs-a :attrs, tuples-a :tuples} a
         {attrs-b :attrs, tuples-b :tuples} b]
     (cond
       (= attrs-a attrs-b)
       (Relation. attrs-a (into (vec tuples-a) tuples-b))
 
-      (and (not (same-keys? attrs-a attrs-b))
-           (seq tuples-a)  ; could be empty because
-           (seq tuples-b)) ; a query short-circuited
+      ;; BEFORE checking same-keys
+      ;; because one rel could have had its resolution shortcircuited
+      (empty? tuples-a) b
+      (empty? tuples-b) a
+
+      (not (same-keys? attrs-a attrs-b))
       (raise "Can’t sum relations with different attrs: " attrs-a " and " attrs-b
              {:error :query/where})
 
       (every? number? (vals attrs-a)) ;; can’t conj into BTSetIter
-      (let [idxb->idxa (vec (for [[sym idx-b] attrs-b]
-                              [idx-b (attrs-a sym)]))
-            tlen    (->> (vals attrs-a) (reduce max) (inc)) 
-            tuples' (persistent!
-                      (reduce
-                        (fn [acc tuple-b]
-                          (let [tuple' (da/make-array tlen)]
-                            (doseq [[idx-b idx-a] idxb->idxa]
-                              (aset tuple' idx-a (#?(:cljs da/aget :clj get) tuple-b idx-b)))
-                            (conj! acc tuple')))
-                        (transient (vec tuples-a))
-                        tuples-b))]
-        (Relation. attrs-a tuples'))
+      (sum-rel* attrs-a tuples-a attrs-b tuples-b)
 
       :else
-      (let [all-attrs (zipmap (keys (merge attrs-a attrs-b)) (range))]
-        (-> (Relation. all-attrs [])
-            (sum-rel a)
+      (let [number-attrs (zipmap (keys attrs-a) (range))]
+        (-> (sum-rel* number-attrs [] attrs-a tuples-a)
             (sum-rel b))))))
 
 (defn prod-rel

test/datascript/test/query_or.cljc

@@ -65,7 +65,16 @@
          (and [2  :age  ?a]
               [?e :name "Oleg"]))
      [?e :age ?a]]
-    #{1 5 4}))
+    #{1 5 4}
+
+    ;; One branch of or short-circuits resolution
+    [(or
+       (and [?e :age 30] ; no matches in db
+            [?e :name ?n])
+       (and [?e :age 20]
+            [?e :name ?n]))
+     [(ground "Ivan") ?n]]
+    #{2 6}))
 
 (deftest test-or-join
   (are [q res] (= (d/q (concat '[:find ?e :where] (quote q)) @test-db)
@@ -83,16 +92,9 @@
             [?e2 :age ?a]))]
     #{1 2 3 4 5 6}
 
+    ;; One branch of or-join short-circuits resolution
     [(or-join [?e ?n]
-       (and [?e :age 30] ; no matches, so this branch short-circuits
-            [?e :name ?n])
-       (and [?e :age 20]
-            [?e :name ?n]))
-     [(ground "Ivan") ?n]]
-    #{2 6}
-
-    [(or
-       (and [?e :age 30] ; no matches, so this branch short-circuits
+       (and [?e :age 30] ; no matches in db
             [?e :name ?n])
        (and [?e :age 20]
             [?e :name ?n]))