Skip to content

Commit

Permalink
Issue 304: Respect binding groups in complex extracted negations (#342)
Browse files Browse the repository at this point in the history
  • Loading branch information
WilliamParker authored Oct 24, 2017
1 parent 68816b0 commit 6b83688
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 6 deletions.
30 changes: 26 additions & 4 deletions src/main/clojure/clara/rules/compiler.clj
Original file line number Diff line number Diff line change
Expand Up @@ -983,16 +983,38 @@
"__"
(gensym))

;; Insert the bindings from ancestors that are used in the negation
;; in the NegationResult fact so that the [:not [NegationResult...]]
;; condition can assert that the facts matching the negation
;; have the necessary bindings.
;; See https://github.com/cerner/clara-rules/issues/304 for more details
;; and a case that behaves incorrectly without this check.
ancestor-bindings-in-negation-expr (set/intersection
(variables-as-keywords negation-expr)
ancestor-bindings)

ancestor-bindings-insertion-form (into {}
(map (fn [binding]
[binding (-> binding
name
symbol)]))
ancestor-bindings-in-negation-expr)

ancestor-binding->restriction-form (fn [b]
(list '= (-> b name symbol)
(list b 'ancestor-bindings)))

modified-expression `[:not {:type ~(if (compiling-cljs?)
'clara.rules.engine/NegationResult
'clara.rules.engine.NegationResult)
:constraints [(~'= ~gen-rule-name ~'gen-rule-name)]}]


:constraints [(~'= ~gen-rule-name ~'gen-rule-name)
~@(map ancestor-binding->restriction-form
ancestor-bindings-in-negation-expr)]}]

generated-rule (cond-> {:name gen-rule-name
:lhs (concat previous-expressions [negation-expr])
:rhs `(clara.rules/insert! (eng/->NegationResult ~gen-rule-name))}
:rhs `(clara.rules/insert! (eng/->NegationResult ~gen-rule-name
~ancestor-bindings-insertion-form))}

;; Propagate properties like salience to the generated production.
(:props production) (assoc :props (:props production))
Expand Down
4 changes: 2 additions & 2 deletions src/main/clojure/clara/rules/engine.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@
(do
;; A marker interface to identify internal facts.
(definterface ISystemFact)
(defrecord NegationResult [gen-rule-name]
(defrecord NegationResult [gen-rule-name ancestor-bindings]
ISystemFact))

:cljs
(do
(defrecord NegationResult [gen-rule-name])
(defrecord NegationResult [gen-rule-name ancestor-bindings])
;; Make NegationResult a "system type" so that NegationResult
;; facts are special-cased when matching productions. This serves
;; the same purpose as implementing the ISystemFact Java interface
Expand Down
16 changes: 16 additions & 0 deletions src/test/clojure/clara/test_rules.clj
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,21 @@
(fire-rules))]
(is (= [{:?l "MCI"}]
(query end-session nested-negation-with-prior-bindings)))
(is (no-system-types? end-session)))

;; Match the nested negation for location ORD but not for MCI.
;; See https://github.com/cerner/clara-rules/issues/304
(let [end-session (-> s-with-nested
(insert
(->WindSpeed 10 "MCI")
(->Temperature 10 "MCI")
(->Cold 20)
(->WindSpeed 20 "ORD")
(->Temperature 20 "ORD"))
(fire-rules))]

(is (= [{:?l "ORD"}]
(query end-session nested-negation-with-prior-bindings)))
(is (no-system-types? end-session)))))

(deftest test-complex-negation-custom-type
Expand Down Expand Up @@ -2169,6 +2184,7 @@
(insert (->Cold 10) (->Hot 10))
(fire-rules)
(query q)))]

(is (= (session->results (mk-session [r q]))
;; Validate that equal salience is handled correctly by
;; the activation-group-sort-fn when the user provides
Expand Down

0 comments on commit 6b83688

Please sign in to comment.