Optimize FindQuery.update() with partial writes - #843
Conversation
|
Thanks for opening, @lh0156! I'll take a look ASAP. |
| return { | ||
| key: ("1" if value else "0") if isinstance(value, bool) else value | ||
| for key, value in document.items() | ||
| } |
There was a problem hiding this comment.
Bool-to-string conversion unreachable after jsonable_encoder processes integers
Medium Severity
In _serialize_update_values for HashModel, the jsonable_encoder call returns booleans as-is because isinstance(False, int) is True in its (str, int, float, type(None)) check. However, the subsequent boolean-to-string conversion on line 2108 checks isinstance(value, bool), which works correctly for Python bool objects. The real issue is that jsonable_encoder processes dict values recursively and can convert certain nested or complex typed booleans inconsistently depending on the Pydantic model's encoder configuration. More importantly, updating a boolean field to False produces "0", which after if value is not None passes through but could be confused with integer 0 by downstream code. This inconsistency between int(0) staying as 0 and bool(False) becoming "0" could cause subtle data type mismatches on read-back if fields are typed as int but store boolean-like values.
Reviewed by Cursor Bugbot for commit 46ec114. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit 0363c8a. Configure here.
| for key in keys: | ||
| pipeline.hset(key, mapping=serialized_values) | ||
| for field_name, ttl in preserved_ttls.get(key, {}).items(): | ||
| pipeline.hexpire(key, ttl, field_name) |
There was a problem hiding this comment.
Missing Field expire on update
High Severity
FindQuery.update() only restores existing positive hash field TTLs after HSET and never applies Field(expire=N) defaults. The old path called save(), which falls back to those defaults when a field has no TTL. After a field expires and Redis deletes it, a later bulk update can recreate the field with no expiry, so values such as session tokens may never expire again.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0363c8a. Configure here.


Summary
Implements #777 by updating matching records without materializing full Pydantic models.
What changed
FT.SEARCH ... NOCONTENTto collect matching keys page by page.HSETmappings forHashModel.JSON.SETcommands forJsonModel, including nested__paths.use_transactionflag and returns the number of updated records.Testing
pytest -q tests --ignore tests/test_benchmarks.py: 265 passed.pytest -q tests/test_hash_model.py tests/test_json_model.py: 147 passed.ruff checkandruff format --checkon changed source/tests.python -m compileall -q aredis_om tests.bandit -q -r aredis_om/model/model.py -s B608.Redis Stack was run locally via Docker Compose for the integration tests; benchmark tests were excluded from the full run.
Note
Medium Risk
Bulk updates now bypass per-instance
save()and use direct Redis writes, so behavior depends on the new serialization/validation path and pipeline semantics; hash field TTL preservation adds extra Redis calls on supported servers.Overview
FindQuery.update()no longer loads full models and callssave()per match. It validates and serializes kwargs once, pagesFT.SEARCHwithNOCONTENTto collect keys (ignoring.only()projections), then applies partialHSETmappings forHashModelorJSON.SETon$.paths (including__nested paths) forJsonModelin a pipeline.use_transactionis honored,FindQuery.dict()now copiesknn, and the method returns the number of updated records (0 when nothing matches or Hash updates would be empty).Hash bulk updates re-apply
HEXPIREfor updated fields when Redis 7.4+ field TTLs were set. Docs show the return value; new unit and integration tests cover validation, pagination, JSON paths, and TTL preservation.Reviewed by Cursor Bugbot for commit 0363c8a. Bugbot is set up for automated code reviews on this repo. Configure here.