Skip to content

Commit

Permalink
#417: Duplicate unifications within the same condition cause rebinding (
Browse files Browse the repository at this point in the history
  • Loading branch information
EthanEChristian authored Apr 29, 2019
1 parent 253e9c8 commit eb8cf88
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
This is a history of changes to clara-rules.

# 0.20.0 SNAPSHOT
* Correct duplicate bindings within the same condition. See [issue 417](https://github.com/cerner/clara-rules/issues/417)

# 0.19.1
* Added a new field to the clara.rules.engine/Accumulator record. This could be a breaking change for any user durability implementations with low-level performance optimizations. See [PR 410](https://github.com/cerner/clara-rules/pull/410) for details.
* Performance improvements for :exists conditions. See [issue 298](https://github.com/cerner/clara-rules/issues/298).
Expand Down
13 changes: 11 additions & 2 deletions src/main/clojure/clara/rules/compiler.clj
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@
(if (empty? exp-seq)
`(deref ~'?__bindings__)
(let [ [exp & rest-exp] exp-seq
compiled-rest (compile-constraints rest-exp equality-only-variables)
variables (into #{}
(filter (fn [item]
(and (symbol? item)
Expand All @@ -264,7 +263,17 @@
exp))
expression-values (remove variables (rest exp))
binds-variables? (and (equality-expression? exp)
(seq variables))]
(seq variables))

;; if we intend on binding any variables at this level of the
;; expression then future layers should not be able to rebind them.
;; see https://github.com/cerner/clara-rules/issues/417 for more info
equality-only-variables (if binds-variables?
(into equality-only-variables
variables)
equality-only-variables)

compiled-rest (compile-constraints rest-exp equality-only-variables)]

(when (and binds-variables?
(empty? expression-values))
Expand Down
41 changes: 41 additions & 0 deletions src/test/common/clara/test_bindings.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,44 @@
fire-rules
(query cold-query)))
"The query results should not be empty for a matching location"))

;; https://github.com/cerner/clara-rules/issues/417
(def-rules-test test-duplicate-unification

{:queries [single-condition [[]
[[?temp <- Temperature (= ?t temperature)
(= ?t -10)]]]
multi-condition-expression [[]
[[?temp <- Temperature (= ?t temperature)
(= ?t -10)]
[ColdAndWindy (< ?t temperature)]]]

multi-condition-equality [[]
[[?temp <- Temperature (= ?t temperature)
(= ?t -14)]
[ColdAndWindy (= ?t temperature)]]]]

:sessions [empty-session [single-condition
multi-condition-expression
multi-condition-equality] {}]}

(let [session (-> empty-session
(insert (->Temperature -10 "MCI"))
(insert (->Temperature 10 "LAX"))
(insert (->Temperature 30 "ATL"))
fire-rules)

multi-condition (-> empty-session
(insert (->Temperature -10 "ATL"))
(insert (->Temperature -14 "MCI"))
(insert (->ColdAndWindy -9 "ATL"))
(insert (->ColdAndWindy -14 "MCI"))
fire-rules)]
(is (= [(->Temperature -10 "MCI")]
(map :?temp (query session single-condition))))

(is (= [(->Temperature -10 "ATL")]
(map :?temp (query multi-condition multi-condition-expression))))

(is (= [(->Temperature -14 "MCI")]
(map :?temp (query multi-condition multi-condition-equality))))))

0 comments on commit eb8cf88

Please sign in to comment.