fix(cypher): parse and apply the full multi-key ORDER BY list, keeping LIMIT (#1334) - #1436
Merged
Conversation
…g LIMIT (#1334) The Cypher request model stored a single order_by expression; on ORDER BY key1 DESC, key2 ASC LIMIT n the parser consumed only key1, left ', key2 ... LIMIT n' unparsed, and the query silently returned the entire result set (6326 rows / 117 KB instead of 5 on the reporter's graph - a token flood straight into agent context). The return clause now models up to CBM_CYPHER_ORDER_KEYS_MAX (8) sort keys with per-key direction (Cypher semantics). Both sort sites - rb_apply_order_by for RETURN and sort_bindings for the WITH pipeline - compare key-by-key with later keys breaking ties. More keys than the modeled maximum is a loud parse error, never a silently dropped remainder. The no-ORDER-BY projection fast path is preserved via order_key_count == 0. Tests: parse-level (2 keys, per-key direction, LIMIT consumed; 9-key over-cap rejected), exec-level RETURN (limit kept, tiebreak ordering, per-key direction), and exec-level WITH pipeline (limit kept). All RED before the fix (row_count 4 instead of 2), GREEN after. Recorded, pre-existing and unchanged: an ORDER BY key that is not part of the projection is silently skipped (single-key main behaves the same); sorting by unprojected properties needs hidden-column projection and is a separate issue. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
enabled auto-merge
August 4, 2026 16:23
The over-cap ORDER BY rejection path (and the two sibling error paths in parse_return_or_with) freed the items array but not the strings inside it - ASan flagged 7 leaked bytes on the diag and Linux legs the moment the new over-cap test exercised the path. Route all three error paths through free_return_clause(), which already releases item strings, CASE expressions, args and the ORDER BY key list. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
1 participant
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.
Fixes #1334.
Root cause
The Cypher request model stored a single
order_byexpression. OnORDER BY key1 DESC, key2 ASC LIMIT nthe parser consumed onlykey1 DESC, left, key2 … LIMIT nunparsed, and the query silently returned the entire result set — 6,326 rows / 117 KB instead of 5 on the reporter's graph, a token flood straight into agent context.Fix
LIMITparses again.rb_apply_order_by(RETURN) andsort_bindings(WITH pipeline) — compare key-by-key, later keys breaking ties.order_key_count == 0).Verification
row_count == 4, expected 2×3), GREEN with the fix, RED again on revert (production code reverted to origin/main with the fix committed, field-dependent tests excluded for compile).cypher175 passed ·mcp189 passed / 2 skipped (pre-existing) ·make -f Makefile.cbm lint-ciclean.Recorded (pre-existing, unchanged)
An ORDER BY key that is not part of the projection is silently skipped — single-key main behaves the same (the reporter's case A sorted by nothing, correctly capped). Sorting by unprojected properties needs hidden-column projection; separate issue.