fix(arcadedb): work around index-scan datetime comparison bug (3 code paths)#3
Conversation
…de paths ArcadeDB's Cypher index-range-scan optimizer (NodeIndexRangeScan) throws a ClassCastException when comparing a stored LocalDateTime against a native Bolt OffsetDateTime parameter, because it has explicit numeric coercion but no equivalent for temporal types. This only triggers when a range index exists on the compared property, so it wasn't caught by earlier testing that didn't build the full index set. This affects three code paths that compare datetime properties without wrapping them in Cypher's datetime(): - graph_data_operations.py: retrieve_episodes() valid_at filter/order - search_filters.py: date_filter_query_constructor() used by valid_at/invalid_at/created_at/expired_at edge filters - graphiti.py: summarize_saga() and _saga_get_previous_episode_uuid() created_at/valid_at filters and ordering Work around by wrapping both sides of the comparison in datetime(...) in the generated Cypher, gated on provider == GraphProvider.ARCADEDB, mirroring the existing Neptune-only branch pattern already used elsewhere in the codebase. The search_filters.py case was silently returning empty results rather than crashing, which is arguably worse. Filed arcadedb#5008 upstream with a suggested engine-level fix (reusing TemporalUtil.fromCoreJavaType(), already used by the ComparisonExpression path, in NodeIndexRangeScan.compareValues()). Verified against a real ArcadeDB 26.8.1-SNAPSHOT instance with the full 28-index set from build_indices_and_constraints(), 3x before/after for each of the 3 code paths.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces workarounds for ArcadeDB's native datetime comparison issues by wrapping datetime properties and parameters in datetime(...) across several queries in graphiti.py, search_filters.py, and graph_data_operations.py. The review feedback highlights that wrapping these indexed properties makes the queries non-sargable, which could lead to performance degradation on larger datasets. Additionally, a type mismatch was identified in graph_data_operations.py where a dynamically constructed query using f-strings is incorrectly annotated as LiteralString instead of str.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -138,9 +157,9 @@ async def retrieve_episodes( | |||
| query_params['source'] = source.name | |||
|
|
|||
| query: LiteralString = ( | |||
There was a problem hiding this comment.
The query variable is annotated as LiteralString, but it is constructed dynamically using f-strings and query_filter (which is also a dynamic string). In Python, f-strings with runtime variables do not satisfy the LiteralString type constraint, which will cause static type checkers like mypy or pyright to report a type mismatch error. We should change the type annotation to str.
| query: LiteralString = ( | |
| query: str = ( |
| if provider == GraphProvider.ARCADEDB and operator not in ( | ||
| ComparisonOperator.is_null, | ||
| ComparisonOperator.is_not_null, | ||
| ): | ||
| value_name = f'datetime({value_name})' | ||
| param_name = f'datetime({param_name})' |
There was a problem hiding this comment.
Wrapping the indexed property (value_name) in the datetime() function makes the query non-sargable. This means ArcadeDB's query planner will not be able to utilize the range index on these properties, potentially falling back to full scans and causing performance degradation on larger datasets. While this is a necessary workaround for the upstream engine bug (arcadedb#5008), please ensure there is a tracking issue or TODO to remove this workaround once the upstream fix is released.
|
Closing this in favor of the upstream engine fix — arcadedb#5008 was fixed in |
1 participant
What
While digging into the
search_interfacePR (#2 — thanks again for the quick review on that one!), I hit aClassCastExceptionfrom ArcadeDB when running queries that comparedatetimeproperties on indexed fields. Turned out to be an engine-level bug, so I've filed arcadedb#5008 upstream with what I found, plus a suggested fix.In the meantime, this PR works around it on the driver side in three places that do unwrapped datetime comparisons in generated Cypher:
graphiti_core/utils/maintenance/graph_data_operations.py—retrieve_episodes()'svalid_atfilter/ordergraphiti_core/search/search_filters.py—date_filter_query_constructor(), used by thevalid_at/invalid_at/created_at/expired_atedge filtersgraphiti_core/graphiti.py—summarize_saga()and_saga_get_previous_episode_uuid()'screated_at/valid_atfilters and orderingThe fix wraps both sides of the comparison in Cypher's
datetime(...), gated onprovider == GraphProvider.ARCADEDB, following the same pattern already used for the Neptune-specific branches elsewhere in the codebase. So this only changes behavior for the ArcadeDB provider — no other backend is touched.Why it wasn't caught earlier
The bug only triggers when a range index exists on the compared property — ArcadeDB's query planner picks a different (buggy) execution path in that case. My earlier testing didn't build out the full index set, so this slipped through. Once I rebuilt against the real 28-index set from
build_indices_and_constraints(), it reproduced consistently.Worth calling out: the
search_filters.pycase doesn't throw — it silently returns empty results instead, which took a bit longer to track down.Testing
Verified against a real ArcadeDB 26.8.1-SNAPSHOT instance (Docker), reproducing each of the 3 code paths 3x before the fix and confirming correct behavior 3x after, using the full index set rather than an isolated one.
Upstream
Filed arcadedb#5008 with the root cause (
NodeIndexRangeScan.compareValues()has explicit numeric-type coercion but no equivalent for temporal types) and a suggested fix (reusingTemporalUtil.fromCoreJavaType(), which theComparisonExpressionpath already uses). Happy to have this driver-side workaround removed once/if that lands — just flagging it's a real engine gap, not something we need to carry long-term.