Merge pull request #11 from KainosSoftwareLtd/VEL-1258-tests-and-rename-of-param-arguments

Vel 1258 tests and rename of param arguments

Author: szymonp-kainos

README.md

@@ -196,6 +196,19 @@ SELECT fhir_valueset_expand('{"id": "issue-types", "filter": "err"}');
 
 SELECT fhir_conformance('{"default": "values"}');
 -- return simple Conformance resource, based on created stores
+
+---
+
+-- use different methods to calculate total elements to improve performance: no _totalMethod or _totalMethod=exact uses standard approach
+SELECT fhir_search('{"resourceType": "Patient", "queryString": "name=smith"}');
+SELECT fhir_search('{"resourceType": "Patient", "queryString": "name=smith&_totalMethod=exact"}');
+
+-- _totalMethod=extimated - faster but 'total' is estimated.
+SELECT fhir_search('{"resourceType": "Patient", "queryString": "name=smith&_totalMethod=estimated"}');
+
+-- _totalMethod=no - fastest but no 'total' is returned.
+SELECT fhir_search('{"resourceType": "Patient", "queryString": "name=smith&_totalMethod=no"}');
+
 ```
 
 ## Contribution

src/fhir/search.litcoffee

@@ -247,10 +247,10 @@ To build search query we need to
         if expr.count != null || expr.page != null
           hsql.limit = expr.count || DEFAULT_RESOURCES_PER_PAGE
 
-        if expr.page != null
+        if expr.page?
           hsql.offset = (expr.count || DEFAULT_RESOURCES_PER_PAGE) * expr.page
 
-        if expr.offset != null
+        if expr.offset?
           hsql.offset = expr.offset
 
         if expr.joins
@@ -397,11 +397,11 @@ we just strip limit, offset, order and rewrite select clause:
       q
 
     get_count = (plv8, honey, query_obj) ->
-      if !query_obj.total_method || query_obj.total_method is "std"
-        query_obj.total_method = "std"
+      if !query_obj.total_method || query_obj.total_method is "exact"
+        query_obj.total_method = "exact"
 
         utils.exec(plv8, countize_query(honey))[0].count
-      else if query_obj.total_method is "improved"
+      else if query_obj.total_method is "estimated"
         sql_query= sql(honey)
         query = sql_query[0].replace /\$(\d+)/g, (match, number) ->
             if typeof sql_query[number] is "string"
@@ -416,7 +416,7 @@ we just strip limit, offset, order and rewrite select clause:
         tmp = plv8.execute(query)
         tmp[0].count_estimate
       else
-        throw new Error("Invalid value of totalMethod. only 'std', 'improved' and 'no' allowed.")
+        throw new Error("Invalid value of totalMethod. only 'exact', 'estimated' and 'no' allowed.")
 
 We cache FHIR meta-data index per connection using plv8 object:
 

test/fhir/query_string_spec.edn

@@ -26,6 +26,15 @@
  [["Patient" "_count=11"]
   {:query "Patient" :count 11}]
 
+ [["Patient" "_totalMethod=exact"]
+  {:query "Patient" :total_method "exact"}]
+
+ [["Patient" "_totalMethod=no"]
+  {:query "Patient" :total_method "no"}]
+
+ [["Patient" "_totalMethod=estimated"]
+  {:query "Patient" :total_method "estimated"}]
+
  [["Patient" "_page=12"]
   {:query "Patient" :page 12}]
 
@@ -34,7 +43,7 @@
    :joins [(chained
             (param {:resourceType "Patient" :name "careprovider" :join "Practitioner"} {:value "$id"})
             (param {:resourceType "Practitioner" :name "name"} {:value "igor"}))]}]
- 
+
  [["Patient" "careprovider:Practitioner.organization:Organization.name=hl7"]
   {:query "Patient"
    :joins [(chained

test/fhir/search/pagination_search.yaml

@@ -63,3 +63,23 @@ queries:
         result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=0&_count=1"
       - path: ['link', 3, 'url']
         result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=3&_count=1"
+
+  - query: {resourceType: 'Encounter', queryString: 'patient=Patient/nicola&_sort=patient&_page=1&_count=1&_totalMethod=exact'}
+    total: 3
+    probes:
+      - path: ['entry', 'length']
+        result: 1
+      - path: ['link', 0, 'url']
+        result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=1&_count=1&_totalMethod=exact"
+      - path: ['link', 1, 'url']
+        result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=2&_count=1&_totalMethod=exact"
+      - path: ['link', 2, 'url']
+        result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=0&_count=1&_totalMethod=exact"
+      - path: ['link', 3, 'url']
+        result: "Encounter/search?patient=Patient/nicola&_sort=patient&_page=3&_count=1&_totalMethod=exact"
+
+  - query: {resourceType: 'Encounter', queryString: 'patient=Patient/nicola&_sort=patient&_page=1&_count=1&_totalMethod=no'}
+    total: _undefined
+    probes:
+      - path: ['link']
+        result: '_undefined'

test/fhir/search_spec.coffee

@@ -8,19 +8,13 @@ test = require('../helpers.coffee')
 
 assert = require('assert')
 
-# plv8.debug = true
 get_in = (obj, path)->
   cur = obj
   cur = cur[item] for item in path when cur
   cur
 
 match = (x)-> (y)-> y.indexOf(x) > -1
 
-# plv8.debug = true
-
-# console.log plv8.execute("SET search_path='user1';")
-# console.log plv8.execute("SHOW search_path;")
-
 FILTER = 'uri'
 FILTER = 'incl'
 FILTER = 'search'
@@ -30,7 +24,6 @@ fs.readdirSync("#{__dirname}/search").filter(match(FILTER)).forEach (yml)->
   describe spec.title, ->
     before ->
       plv8.execute("SET plv8.start_proc = 'plv8_init'")
-      # plv8.debug = true
 
       for res in spec.resources
         schema.fhir_create_storage(plv8, resourceType: res)
@@ -56,18 +49,21 @@ fs.readdirSync("#{__dirname}/search").filter(match(FILTER)).forEach (yml)->
         plv8.execute "SET enable_seqscan = OFF;" if (q.indexed or q.indexed_order)
 
         res = search.fhir_search(plv8, q.query)
-        # console.log(JSON.stringify(res))
         explain = JSON.stringify(search.fhir_explain_search(plv8, q.query))
-        # console.log(JSON.stringify(search.fhir_search_sql(plv8, q.query)))
 
         plv8.execute "SET enable_seqscan = ON;" if (q.indexed or q.indexed_order)
 
         if q.total || q.total == 0
-          assert.equal(res.total, q.total)
-
-        (q.probes || []).forEach (probe)-> assert.equal(get_in(res, probe.path), probe.result)
-
-        # console.log(explain)
+          if q.total == "_undefined"
+            assert.equal(res.total, undefined)
+          else
+            assert.equal(res.total, q.total)
+
+        (q.probes || []).forEach (probe)->
+          if probe.result == "_undefined"
+            assert.equal(get_in(res, probe.path), undefined)
+          else
+            assert.equal(get_in(res, probe.path), probe.result)
 
         if q.indexed
           assert(explain.indexOf("Index Cond") > -1, "Should be indexed but #{explain}")