Skip to content

fix(cypher): apply distinct before return limits#906

Merged
DeusData merged 1 commit into
DeusData:mainfrom
pcristin:fix/cypher-distinct-order-limit-pr
Jul 9, 2026
Merged

fix(cypher): apply distinct before return limits#906
DeusData merged 1 commit into
DeusData:mainfrom
pcristin:fix/cypher-distinct-order-limit-pr

Conversation

@pcristin

@pcristin pcristin commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Apply RETURN DISTINCT before ORDER BY, SKIP, and LIMIT post-processing.
  • Preserve the simple RETURN ... LIMIT fast path only for non-DISTINCT, non-ordered returns.
  • Add regressions for RETURN DISTINCT with ORDER BY/LIMIT, LIMIT only, and ORDER BY/SKIP/LIMIT.

Root Cause

RETURN rows were sorted and truncated before de-duplication. When duplicate projected rows appeared before the limit boundary, those duplicates consumed the limit and the final DISTINCT pass returned too few rows.

Test Plan

  • make -f Makefile.cbm build/c/test-runner
  • ./build/c/test-runner 2>&1 | awk '/=== cypher ===/{show=1} show{print} /cypher_parse_unwind_var/{exit}'
    • Passed the Cypher regression window, including the new issue 873 tests.
  • git diff --check origin/main..HEAD
  • scripts/test.sh
    • Failed in the existing incremental/MCP tooling suite with repeated pipeline.err phase=dump failures and an AddressSanitizer leak summary.
    • The observed failures were outside the Cypher query execution path changed here.
  • scripts/lint.sh
    • Failed after installing clang-tidy, clang-format, and cppcheck.
    • clang-tidy reports repository-wide warnings-as-errors outside this change.
    • cppcheck reports src/pipeline/pass_pkgmap.c:421 null-pointer warning and an internal AST error at internal/cbm/extract_defs.c:3386.

Notes

Fixes #873
Refs #594

@pcristin pcristin force-pushed the fix/cypher-distinct-order-limit-pr branch from edeb189 to 417cd1c Compare July 6, 2026 07:42
@pcristin pcristin marked this pull request as ready for review July 6, 2026 07:43
@pcristin pcristin requested a review from DeusData as a code owner July 6, 2026 07:43
@pcristin pcristin force-pushed the fix/cypher-distinct-order-limit-pr branch 2 times, most recently from 218b103 to 76af130 Compare July 7, 2026 14:50
@DeusData DeusData added bug Something isn't working cypher Cypher query language parser/executor bugs priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 7, 2026
@DeusData

DeusData commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the #873 Cypher fix. Triage: high-priority query correctness bug.

Review focus: DISTINCT must happen before ORDER BY/SKIP/LIMIT for the affected shapes, while the non-DISTINCT fast path stays intact. The reported unrelated suite failures should stay clearly separated from this diff during review.

@pcristin pcristin force-pushed the fix/cypher-distinct-order-limit-pr branch from 76af130 to 7dd67bc Compare July 8, 2026 06:49
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 8, 2026
Signed-off-by: Pcristin <xxxokzxxx@protonmail.com>
@pcristin pcristin force-pushed the fix/cypher-distinct-order-limit-pr branch from 7dd67bc to 6c7fe78 Compare July 9, 2026 06:40
@DeusData DeusData merged commit fb21514 into DeusData:main Jul 9, 2026
19 checks passed
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Merged — thank you! Clean root-cause fix on the DISTINCT/ORDER BY/LIMIT ordering, and the regressions covering all three clause combinations are exactly the guard we want. Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cypher Cypher query language parser/executor bugs priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

query_graph: RETURN DISTINCT … ORDER BY … LIMIT still under-returns on v0.8.1 (regression / incomplete fix for #237)

2 participants