fix(persistence): chained search honors comparator prefixes, type-guarded - #619
Open
angela-helios wants to merge 1 commit into
Open
fix(persistence): chained search honors comparator prefixes, type-guarded#619angela-helios wants to merge 1 commit into
angela-helios wants to merge 1 commit into
Conversation
…rded A chained search with a comparator prefix silently returned zero rows on SQLite: resolve_chain forced an Eq prefix onto the raw value, so patient.birthdate=le1956-07-14 compared dates against the literal string 'le1956-07-14'. Postgres had the inverse bug: it parsed the prefix unconditionally, so a chained string value beginning with a valid prefix pair — family=Levine — lost its head to le + 'vine'. Both sides now strip the prefix only when the parsed chain's terminal parameter type admits one (number/date/quantity, via SearchPrefix::is_valid_for), which is exactly how direct search behaves. The terminal SQL arms already honored value.prefix; only the call sites were wrong. Covered by a test driving resolve_chain through le/gt dates (the issue's repro, including gt1900 matching everyone) and the Levine-stays-whole string guard. Closes #258
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #258.
The silent-empty from the issue, plus its mirror image found on the way:
SearchValue::eqonto the raw chained value, sopatient.birthdate=le1956-07-14compared dates against the literal stringle1956-07-14— zero rows, no error.gt1900-01-01returning nothing was exactly this.SearchValue::parse, so a chained string value starting with a valid prefix pair —family=Levine— lost its head tole+vine.Both call sites now strip the prefix only when the parsed chain's terminal parameter type admits one (number/date/quantity via
SearchPrefix::is_valid_for) — the same rule direct search applies. The terminal SQL arms (build_date_condition/build_number_condition) already honoredvalue.prefix; only the call sites were wrong, so the fix is two guarded parses.Test drives
resolve_chainthrough the issue's repro (le includes,gt1900matches everyone, le on an earlier date excludes) and the Levine-stays-whole guard. Full-p helios-persistence --features sqlitesuite green (792), clippy clean on both backends. The ES/Mongo chain paths ride different code (registry-driven, no raw-eq call site) — the multi-backend verification pass tracked in #519 is the natural place to sweep them with the same repro.