Skip to content

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

Open
pcristin wants to merge 1 commit into
DeusData:mainfrom
pcristin:fix/cypher-distinct-order-limit-pr
Open

fix(cypher): apply distinct before return limits#906
pcristin wants to merge 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.

Signed-off-by: Pcristin <xxxokzxxx@protonmail.com>
@pcristin pcristin force-pushed the fix/cypher-distinct-order-limit-pr branch from 76af130 to 7dd67bc Compare July 8, 2026 06:49
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