Skip to content

Commit

Permalink
Handle special case of cardinality [0..0] and wildcard values
Browse files Browse the repository at this point in the history
When handling refinements, we can transform wildcard values to a count of attributes query, such that a search is performed for results with >0 attributes matching the query. However, when used in conjunction with a [0..0] cardinality, we must reverse this wildcard filter so that we actually exclude results with >0 attributes. When the expression contains a minimum cardinality of zero, we are careful to avoid adding an attribute count query, and for each value, simply exclude results with an attribute count greater than the maximum requested. Fixes #46.
  • Loading branch information
wardle committed Apr 1, 2023
1 parent 85505b1 commit 72edd81
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
14 changes: 8 additions & 6 deletions src/com/eldrix/hermes/impl/ecl.clj
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,10 @@
(search/q-or (map #(search/q-attribute-in-set % (realise-concept-ids ctx incl)) attribute-concept-ids)))))

(defn- parse-attribute--expression
[ctx cardinality reverse-flag? attribute-concept-ids loc]
[ctx {min-value :min-value max-value :max-value :as cardinality} reverse-flag? attribute-concept-ids loc]
(let [sub-expression (zx/xml1-> loc :subExpressionConstraint (partial parse-subexpression-constraint ctx))
attribute-query (make-attribute-query ctx sub-expression attribute-concept-ids)]
attribute-query (when-not (and cardinality (zero? min-value)) (make-attribute-query ctx sub-expression attribute-concept-ids))
cardinality-queries (when cardinality (seq (remove nil? (map #(search/q-attribute-count % min-value max-value) attribute-concept-ids))))]
(cond
;; we are not trying to implement edge case of an expression containing both cardinality and reversal, at least not yet
;; see https://confluence.ihtsdotools.org/display/DOCECL/6.3+Cardinality for how it *should* work
Expand All @@ -476,10 +477,11 @@
(process-dotted ctx (realise-concept-ids ctx sub-expression) [(search/q-concept-ids attribute-concept-ids)])

;; if we have cardinality, add a clause to ensure we have the right count for those properties
cardinality
(search/q-and (filter identity
(conj (map #(search/q-attribute-count % (:min-value cardinality) (:max-value cardinality)) attribute-concept-ids)
attribute-query)))
(and attribute-query cardinality-queries)
(search/q-and (conj cardinality-queries attribute-query))

cardinality-queries
cardinality-queries

:else
attribute-query)))
Expand Down
2 changes: 1 addition & 1 deletion src/com/eldrix/hermes/impl/search.clj
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ items."
(q-not (MatchAllDocsQuery.) (IntPoint/newRangeQuery field 1 Integer/MAX_VALUE))

(and (zero? minimum) (pos? maximum))
(q-not (MatchAllDocsQuery.) (IntPoint/newRangeQuery field 1 (int maximum))))))
(q-not (MatchAllDocsQuery.) (IntPoint/newRangeQuery field (int (inc maximum)) Integer/MAX_VALUE)))))

(defn q-term [s] (make-tokens-query s))

Expand Down
12 changes: 11 additions & 1 deletion test/com/eldrix/hermes/ecl_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
(:require [clojure.core.async :as a]
[clojure.set :as set]
[clojure.spec.test.alpha :as stest]
[clojure.test :refer [deftest is use-fixtures run-tests]]
[clojure.test :refer [deftest is use-fixtures run-tests testing]]
[com.eldrix.hermes.core :as hermes]
[com.eldrix.hermes.impl.store :as store]
[com.eldrix.hermes.rf2]
Expand Down Expand Up @@ -164,6 +164,16 @@
(is (seq results) "Invalid results")
(is (every? true? results) "Invalid results")))))

(deftest ^:live test-cardinality
(testing "Cardinality 0..1 and 1..1"
(let [r0-1 (hermes/expand-ecl *svc* "<<24700007: [0..1] 370135005=*") ;;370135005 = pathological process
r1-1 (hermes/expand-ecl *svc* "<<24700007: [1..1] 370135005=*")]
(is (seq r1-1) "Multiple sclerosis and descendants should have attribute of 'pathological process'")
(is (>= (count r0-1) (count r1-1)) "Results for cardinality 0..1 should be the same, more than 1..1")
(is (set/subset? (set r1-1) (set r0-1)) "Results for cardinality 1..1 should a subset of 0..1")))
(testing "Zero cardinality"
(let [results (hermes/expand-ecl *svc* "<<24700007: [0..0] 246075003=*")] ;; 246075003 = causative agent.
(is (seq results) "Invalid result. Multiple sclerosis and descendants should not have attributes of 'causative agent'"))))

(comment
(def ^:dynamic *svc* (hermes/open "snomed.db"))
Expand Down

0 comments on commit 72edd81

Please sign in to comment.