feat(history): add typed inherent methods and JSON filter to SqliteBackedHistory#1112
Open
eitsupi wants to merge 8 commits into
Open
feat(history): add typed inherent methods and JSON filter to SqliteBackedHistory#1112eitsupi wants to merge 8 commits into
eitsupi wants to merge 8 commits into
Conversation
eitsupi
force-pushed
the
feat/typed-history-extra-info
branch
from
79faf98 to
a04be41
Compare
Add `save_with_extra`, `load_with_extra`, and `search_with_extra` as typed inherent methods on `SqliteBackedHistory`. These methods allow downstream crates to round-trip custom `more_info` types through the SQLite backend without going through the `History` trait. Also add `more_info_json` to `SearchFilter` for SQL-level JSON filtering using SQLite's `json_extract()`. This avoids fetching all rows and filtering in Rust when only a subset matching a JSON path condition is needed. Boolean JSON values compare as integers (`"1"` = true, `"0"` = false) because SQLite's `json_extract` maps JSON booleans to integers. The `History` trait itself remains unchanged (non-breaking). Motivation: downstream crates that hold `SqliteBackedHistory` directly (rather than `Box<dyn History>`) can now store and query typed metadata alongside history entries. Crates using `FileBackedHistory` or `Box<dyn History>` are unaffected.
…r booleans History::count() and History::search() now clear more_info_json before calling construct_query, matching the documented contract that non-typed trait methods do not evaluate JSON filters. For more_info_json filter values of "true" or "false", use SQLite's json_type() instead of CAST(json_extract(...) AS TEXT) to match JSON boolean values exactly. This prevents false matches against integer 1/0 and string "true"/"false". Other filter values continue to use CAST.
…alue enum Add `JsonFilterValue` enum with `Null`, `Bool`, `Integer`, `Real`, and `Text` variants so that `SearchFilter::more_info_json` filters are type-precise. Each variant generates SQL using `json_type()` to prevent collisions between JSON booleans, integers, and strings (e.g. `Bool(true)` no longer matches integer `1` or string `"true"`). Export `JsonFilterValue` from the crate root. Add collision tests covering every scalar type combination. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add json_filter_value_real_vs_integer to verify Real(f) does not collide with Integer values sharing the same numeric representation, and json_filter_value_null_vs_missing_path to verify Null matches JSON null at a path but not SQL-NULL more_info or an absent path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
json_filter_value_real_vs_integer: add an integer row at $.score to verify Real(1.0) excludes it (json_type 'integer' != 'real') while Integer(1) correctly matches it but not the real rows. json_filter_value_null_vs_missing_path: add a row whose non-null JSON has no $.val key (WithoutVal struct) to verify JsonFilterValue::Null excludes it alongside the SQL-NULL more_info row. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eitsupi
force-pushed
the
feat/typed-history-extra-info
branch
from
a04be41 to
6f3efef
Compare
Keep BoxedNamedParams as &'static str keys (unchanged from upstream) to
avoid adding .to_string() to every existing params.push call. Dynamic
JSON filter parameters (":json_path_N" etc.) are collected in a separate
OwnedNamedParams vec and merged at the end of construct_query.
Also remove the spurious Vec<&str> type annotation on `wheres`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
History::save now delegates to save_with_extra rather than a private save_impl, removing the unnecessary indirection layer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
cool. i remember adding more_info thinking someday someone would use it. what are you using it for? |
Contributor
Author
My priorities were distinguishing between special meta-commands available within the REPL and recording the REPL's output. Furthermore, I imagine this would be useful for this tool with REPL that can connect to various databases, such as for distinguishing between different databases. |
Contributor
|
cool. thanks for sharing. |
Contributor
Author
|
My first use case: eitsupi/arf#236 |
Contributor
|
@kronberger-droid I'd probably wait on this one too until after the release unless you feel strongly about landing it now. |
Collaborator
|
Yeah agreed. |
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
3 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.
Summary
Three typed inherent methods are added to
SqliteBackedHistory(save_with_extra,load_with_extra,search_with_extra) for round-trippingmore_infowithout type erasure.History::savenow delegates tosave_with_extra, so both paths share the same implementation. A newJsonFilterValueenum andSearchFilter::more_info_jsonfield enable type-precise filtering by JSON path at the SQL level. TheHistorytrait is unchanged.Motivation
#1011 made
HistoryItemgeneric overExtraInfoand exposedHistoryItemExtraInfopublicly. However, theHistorytrait still uses the type-erasedHistoryItem<IgnoreAllExtraInfo>, so anymore_infowritten through the trait is silently discarded on the next read. The generics are in place, but there is no usable end-to-end path to preserve custom metadata.Downstream crates that want to persist typed metadata in
more_infonecessarily holdSqliteBackedHistorydirectly, sinceFileBackedHistoryhas nomore_infosupport at all. Typed inherent methods onSqliteBackedHistorymodel this capability boundary precisely, without changing theHistorytrait.