Fix: Restore correct column naming and SKIP/LIMIT behavior

[DecisionNerd]

Description

Fixes critical regression introduced by WITH clause implementation that broke 20 integration tests. The bugs were in the executor's result handling logic for column naming and SKIP/LIMIT operations.

Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ New feature
• 💥 Breaking change
• 📚 Documentation update
• 🔧 Configuration/infrastructure change
• ♻️ Refactoring
• ✅ Tests (adding or updating tests)

Related Issues

Fixes the regression introduced in commit 0ddfa48 (WITH clause implementation).

Impact: 20 integration tests failing (5.7% failure rate)

Changes Made

Bug 1: Column Naming (13 test failures)

Problem: Return items without explicit aliases were using variable names (e.g., "n") instead of the established col_N convention (e.g., "col_0").

Root cause: _execute_project() had logic that returned variable names for unnamed items:

elif isinstance(return_item.expression, Variable):
    key = return_item.expression.name  # Returns "n" instead of "col_0"

Fix: Simplified to always use col_{i} for unnamed items:

if return_item.alias:
    key = return_item.alias
else:
    key = f"col_{i}"  # Always use col_N

Bug 2: Empty Results for SKIP/LIMIT (7 test failures)

Problem: Queries with SKIP or LIMIT returned empty results.

Root cause: Executor checked if LAST operator was Project, but SKIP/LIMIT come after Project in the pipeline.

Fix: Changed to check if ANY operator is Project/Aggregate:

if not any(isinstance(op, (Project, Aggregate)) for op in operators):
    return []

Bug 3: TCK Test Collection

Problem: pytest_plugins in non-top-level conftest caused collection error.

Fix: Moved pytest_plugins declaration to tests/conftest.py.

Testing

Test Coverage

• Unit tests added/updated
• Integration tests added/updated
• All existing tests pass
• Coverage maintained or improved

Test Commands Run

# All unit tests
pytest tests/unit/ -v
# Result: 215/215 passing (100%)

# All integration tests  
pytest tests/integration/ -v
# Result: 136/136 passing (100%) - was 116/136 before fix

# TCK collection
pytest --collect-only tests/tck/
# Result: 3,854 tests collected - was collection error before

Test Results

Category	Before	After	Status
Unit tests	215/215 (100%)	215/215 (100%)	✅ No regression
Integration tests	116/136 (85.3%)	136/136 (100%)	✅ Fixed
TCK tests	Collection error	3,854 collected	✅ Fixed

Checklist

Code Quality

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have run ruff format . and ruff check .
• I have run mypy src/ (if type hints added/changed)

Testing

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally
• I have checked that code coverage has not decreased
• I have tested on multiple Python versions (if relevant)

Documentation

• I have updated the documentation (if needed)
• I have updated the CHANGELOG.md (if user-facing change)
• I have added docstrings to new functions/classes
• I have updated the Cypher guide (if adding Cypher features) - N/A (bug fix)

Compliance

• My changes follow openCypher specification (if applicable)
• I have checked TCK compliance impact (if Cypher changes)
• No breaking changes to existing APIs (or documented in description)

Screenshots/Examples

Before fix:

db.execute('MATCH (n) RETURN n')
# Result: [{'n': NodeRef(...)}]  ❌ Wrong

db.execute('MATCH (n) RETURN n LIMIT 2')
# Result: []  ❌ Empty

After fix:

db.execute('MATCH (n) RETURN n')
# Result: [{'col_0': NodeRef(...)}]  ✅ Correct

db.execute('MATCH (n) RETURN n LIMIT 2')
# Result: [{'col_0': NodeRef(...)}, {'col_0': NodeRef(...)}]  ✅ Correct

Performance Impact

• No performance impact
• Performance improved
• Performance may be affected

Performance notes: Minimal impact. Changed from single isinstance() check to any() loop over operators list (typically 3-5 operators), which is negligible.

Breaking Changes

• No breaking changes

This fix restores the original behavior, so it's actually fixing a breaking change that was introduced.

Additional Context

Why This Happened

The WITH clause implementation refactored the planner's query processing, which inadvertently changed how the executor handled return items. The bugs suggest that:

1. Column naming logic may have been added to support WITH variable bindings
2. The "check last operator" logic didn't account for SKIP/LIMIT being added after Project

Documentation Created

• docs/session-2026-01-31-regression-fix.md (335 lines) - Detailed bug analysis
• docs/session-summary.md (339 lines) - Session overview and next steps

Lessons Learned

1. Integration tests are critical - Unit tests all passed but didn't catch the regression
2. API contracts must be preserved - The col_N naming was established behavior
3. Test edge cases - Simple queries worked, but SKIP/LIMIT exposed the bug

Reviewer Notes

Areas to focus on:

1. Verify the column naming logic is correct for all cases:

   • Simple variables: RETURN n → col_0 ✓
   • With alias: RETURN n AS person → person ✓
   • Complex expressions: RETURN n.name → col_0 ✓
   • Multiple items: RETURN n, n.name → col_0, col_1 ✓

2. Verify SKIP/LIMIT work correctly:

   • With RETURN: RETURN n LIMIT 2 → 2 results ✓
   • Without RETURN: CREATE (n) → empty results ✓

3. All 351 tests pass (215 unit + 136 integration)

---

Fixes the regression from WITH clause implementation. Ready for review!

[coderabbitai]

Important

Review skipped

Too many files!

This PR contains 264 files, which is 114 over the limit of 150.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

• 🔍 Trigger a full review

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)

Validation error: Unrecognized key(s) in object: 'python', 'ignore', 'tools', 'file_types', 'comments', 'thresholds', 'integrations', 'notifications'

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/integration-test-regression

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}