Skip to content

fix(persistence): ES _lastUpdated honours ne/sa/eb/ap and OR-lists #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smunini merged 1 commit into from
Sep 2, 2026
Merged

fix(persistence): ES _lastUpdated honours ne/sa/eb/ap and OR-lists #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 99 additions & 98 deletions crates/persistence/src/backends/elasticsearch/search/parameter_handlers/date.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,113 +4,67 @@ use serde_json::{Value, json};

use crate::types::SearchPrefix;

/// Builds an ES query clause for a date search parameter.
pub fn build_clause(name: &str, value: &str, prefix: SearchPrefix) -> Option<Value> {
let range_condition = match prefix {
SearchPrefix::Eq => {
// Equality accounting for precision:
// "2024-01-15" matches the full day
let (lower, upper) = date_precision_range(value);
json!({
"range": {
"search_params.date.value": {
"gte": lower,
"lt": upper
}
}
})
}
SearchPrefix::Ne => {
let (lower, upper) = date_precision_range(value);
return Some(json!({
"nested": {
"path": "search_params.date",
"query": {
"bool": {
"must": [
{ "term": { "search_params.date.name": name } }
],
"must_not": [
{
"range": {
"search_params.date.value": {
"gte": lower,
"lt": upper
}
}
}
]
}
}
}
}));
}
SearchPrefix::Gt | SearchPrefix::Sa => {
let (_, upper) = date_precision_range(value);
json!({
"range": {
"search_params.date.value": {
"gte": upper
}
}
})
}
SearchPrefix::Lt | SearchPrefix::Eb => {
let (lower, _) = date_precision_range(value);
json!({
"range": {
"search_params.date.value": {
"lt": lower
}
}
})
/// A precision-aware comparison on one ES date field, ready to be wrapped in
/// whatever query shape the caller needs (nested for indexed parameters, a
/// bare top-level clause for `_lastUpdated`).
#[derive(Debug)]
pub(crate) enum DateRange {
/// `{ "range": { field: bounds } }` — the value must fall inside.
Within(Value),
/// `{ "range": { field: bounds } }` — the value must fall *outside*
/// (`ne`). The caller negates it with `must_not` so the negation is
/// applied at the right level of the enclosing query.
Outside(Value),
}

/// Builds the `range` comparison for `field` at the value's inherent
/// precision: `eq` at day precision means `[day, day+1)`, `ne` its
/// complement, `gt`/`sa` start strictly after the whole period, `lt`/`eb`
/// end strictly before it, and `le` reaches the end of the period.
///
/// A full-precision instant (`2024-01-15T10:00:00Z`) has no period, so it
/// falls back to scalar comparison rather than an empty half-open range.
pub(crate) fn field_range(field: &str, value: &str, prefix: SearchPrefix) -> DateRange {
let (lower, upper) = date_precision_range(value);
let degenerate = lower == upper;
let range = |bounds: Value| json!({ "range": { field: bounds } });

match prefix {
// `ap` on a date is the precision range itself: ES has no fuzzy
// date matching, and the implied period is the natural tolerance.
SearchPrefix::Eq | SearchPrefix::Ap if degenerate => {
DateRange::Within(range(json!({ "gte": lower, "lte": lower })))
}
SearchPrefix::Ge => {
let (lower, _) = date_precision_range(value);
json!({
"range": {
"search_params.date.value": {
"gte": lower
}
}
})
SearchPrefix::Eq | SearchPrefix::Ap => {
DateRange::Within(range(json!({ "gte": lower, "lt": upper })))
}
SearchPrefix::Le => {
let (_, upper) = date_precision_range(value);
json!({
"range": {
"search_params.date.value": {
"lt": upper
}
}
})
SearchPrefix::Ne if degenerate => {
DateRange::Outside(range(json!({ "gte": lower, "lte": lower })))
}
SearchPrefix::Ap => {
// Approximately: ±10% of the precision range
let (lower, upper) = date_precision_range(value);
// For approximate, we use the range itself (ES handles fuzzy matching)
json!({
"range": {
"search_params.date.value": {
"gte": lower,
"lt": upper
}
}
})
SearchPrefix::Ne => DateRange::Outside(range(json!({ "gte": lower, "lt": upper }))),
SearchPrefix::Gt | SearchPrefix::Sa if degenerate => {
DateRange::Within(range(json!({ "gt": lower })))
}
SearchPrefix::Gt | SearchPrefix::Sa => DateRange::Within(range(json!({ "gte": upper }))),
SearchPrefix::Lt | SearchPrefix::Eb => DateRange::Within(range(json!({ "lt": lower }))),
SearchPrefix::Ge => DateRange::Within(range(json!({ "gte": lower }))),
SearchPrefix::Le if degenerate => DateRange::Within(range(json!({ "lte": lower }))),
SearchPrefix::Le => DateRange::Within(range(json!({ "lt": upper }))),
}
}

/// Builds an ES query clause for an indexed date search parameter.
pub fn build_clause(name: &str, value: &str, prefix: SearchPrefix) -> Option<Value> {
let name_term = json!({ "term": { "search_params.date.name": name } });
let bool_body = match field_range("search_params.date.value", value, prefix) {
DateRange::Within(range) => json!({ "must": [name_term, range] }),
DateRange::Outside(range) => json!({ "must": [name_term], "must_not": [range] }),
};

Some(json!({
"nested": {
"path": "search_params.date",
"query": {
"bool": {
"must": [
{ "term": { "search_params.date.name": name } },
range_condition
]
}
}
"query": { "bool": bool_body }
}
}))
}
Expand Down Expand Up @@ -206,4 +160,51 @@ mod tests {
assert!(s.contains("gte"));
assert!(s.contains("2024-01-16")); // starts after precision range
}

#[test]
fn ne_is_the_negated_precision_range() {
let clause = build_clause("birthdate", "2024-01-15", SearchPrefix::Ne).unwrap();
let bool_body = &clause["nested"]["query"]["bool"];
assert_eq!(
bool_body["must"][0]["term"]["search_params.date.name"],
"birthdate"
);
let range = &bool_body["must_not"][0]["range"]["search_params.date.value"];
assert_eq!(range["gte"], "2024-01-15");
assert_eq!(range["lt"], "2024-01-16");
}

#[test]
fn sa_and_eb_mirror_gt_and_lt_on_the_whole_period() {
let sa = field_range("f", "2024-01", SearchPrefix::Sa);
let DateRange::Within(sa) = sa else {
panic!("sa must be a plain range: {sa:?}")
};
assert_eq!(sa["range"]["f"], json!({ "gte": "2024-02-01" }));

let eb = field_range("f", "2024-01", SearchPrefix::Eb);
let DateRange::Within(eb) = eb else {
panic!("eb must be a plain range: {eb:?}")
};
assert_eq!(eb["range"]["f"], json!({ "lt": "2024-01-01" }));
}

#[test]
fn full_precision_instant_is_scalar_not_an_empty_range() {
let instant = "2024-01-15T10:00:00Z";
let DateRange::Within(eq) = field_range("f", instant, SearchPrefix::Eq) else {
panic!("eq must be a plain range")
};
assert_eq!(eq["range"]["f"], json!({ "gte": instant, "lte": instant }));

let DateRange::Within(gt) = field_range("f", instant, SearchPrefix::Gt) else {
panic!("gt must be a plain range")
};
assert_eq!(gt["range"]["f"], json!({ "gt": instant }));

let DateRange::Within(le) = field_range("f", instant, SearchPrefix::Le) else {
panic!("le must be a plain range")
};
assert_eq!(le["range"]["f"], json!({ "lte": instant }));
}
}
116 changes: 87 additions & 29 deletions crates/persistence/src/backends/elasticsearch/search/query_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,37 +268,38 @@ impl<'a> EsQueryBuilder<'a> {
}
}

/// Builds a clause for the _lastUpdated special parameter.
/// Builds a clause for the `_lastUpdated` special parameter.
///
/// Reuses the precision-aware date logic that indexed date parameters
/// get, against the top-level `last_updated` field: `eq` at day
/// precision means `[day, day+1)`, `ne` its complement, `sa`/`eb` mirror
/// `gt`/`lt` on the whole period, and comma-separated values OR together
/// (#892). Previously every value was folded into one `range` map, so
/// `ne`/`sa`/`eb`/`ap` degraded to `eq` and a second value overwrote the
/// first.
fn build_last_updated_clause(&self, param: &SearchParameter) -> Option<Value> {
let mut range = serde_json::Map::new();
for value in &param.values {
match value.prefix {
SearchPrefix::Eq => {
range.insert("gte".to_string(), json!(value.value));
range.insert("lte".to_string(), json!(value.value));
}
SearchPrefix::Gt => {
range.insert("gt".to_string(), json!(value.value));
}
SearchPrefix::Lt => {
range.insert("lt".to_string(), json!(value.value));
}
SearchPrefix::Ge => {
range.insert("gte".to_string(), json!(value.value));
}
SearchPrefix::Le => {
range.insert("lte".to_string(), json!(value.value));
}
_ => {
range.insert("gte".to_string(), json!(value.value));
range.insert("lte".to_string(), json!(value.value));
let mut clauses: Vec<Value> = param
.values
.iter()
.map(
|value| match date::field_range("last_updated", &value.value, value.prefix) {
date::DateRange::Within(range) => range,
date::DateRange::Outside(range) => {
json!({ "bool": { "must_not": [range] } })
}
},
)
.collect();

match clauses.len() {
0 => None,
1 => clauses.pop(),
_ => Some(json!({
"bool": {
"should": clauses,
"minimum_should_match": 1
}
}
}
if range.is_empty() {
None
} else {
Some(json!({ "range": { "last_updated": Value::Object(range) } }))
})),
}
}

Expand Down Expand Up @@ -451,6 +452,63 @@ mod tests {
}
}

fn last_updated_query(values: Vec<SearchValue>) -> Value {
let query = SearchQuery::new("Patient").with_parameter(SearchParameter {
name: "_lastUpdated".to_string(),
param_type: SearchParamType::Date,
modifier: None,
values,
chain: vec![],
components: vec![],
});
let builder = EsQueryBuilder::new("acme", "Patient", "hfs_acme_patient".to_string());
builder.build(&query).body["query"]["bool"]["must"][0].clone()
}

#[test]
fn last_updated_eq_is_a_day_range() {
let clause = last_updated_query(vec![SearchValue::eq("2026-09-01")]);
assert_eq!(
clause["range"]["last_updated"],
json!({ "gte": "2026-09-01", "lt": "2026-09-02" })
);
}

#[test]
fn last_updated_ne_is_negated_not_eq() {
// #892: `ne` fell into the default arm and matched exactly the day.
let clause = last_updated_query(vec![SearchValue::new(SearchPrefix::Ne, "2026-09-01")]);
assert_eq!(
clause["bool"]["must_not"][0]["range"]["last_updated"],
json!({ "gte": "2026-09-01", "lt": "2026-09-02" })
);
}

#[test]
fn last_updated_sa_and_eb_exclude_the_named_period() {
let sa = last_updated_query(vec![SearchValue::new(SearchPrefix::Sa, "2026-09-01")]);
assert_eq!(sa["range"]["last_updated"], json!({ "gte": "2026-09-02" }));

let eb = last_updated_query(vec![SearchValue::new(SearchPrefix::Eb, "2026-09-01")]);
assert_eq!(eb["range"]["last_updated"], json!({ "lt": "2026-09-01" }));
}

#[test]
fn last_updated_or_list_keeps_every_value() {
// #892: a second value overwrote the first in the single range map.
let clause = last_updated_query(vec![
SearchValue::eq("2026-09-01"),
SearchValue::eq("2026-09-03"),
]);
let should = clause["bool"]["should"]
.as_array()
.expect("OR list -> bool.should");
assert_eq!(should.len(), 2);
assert_eq!(clause["bool"]["minimum_should_match"], 1);
assert_eq!(should[0]["range"]["last_updated"]["gte"], "2026-09-01");
assert_eq!(should[1]["range"]["last_updated"]["gte"], "2026-09-03");
}

fn not_param(values: Vec<SearchValue>) -> SearchQuery {
SearchQuery::new("Patient").with_parameter(SearchParameter {
name: "language".to_string(),
Expand Down
Loading
Loading