Summary of Changes

Hello @lvca, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the OpenCypher implementation by addressing numerous compliance issues identified through TCK tests. It introduces a new list slicing feature, refines the parsing and evaluation of various expressions, and improves the robustness of built-in functions. Additionally, it tightens semantic validation rules and standardizes result formatting, contributing to a more stable and compliant Cypher query engine.

Highlights

• Enhanced Expression Parsing and Evaluation: Refactored the Cypher expression builder to correctly parse complex expressions, including top-level comparisons, arithmetic operations, and postfix expressions (like list slicing). The ArithmeticExpression now supports list concatenation and element appending/prepending using the + operator. Function calls are now dynamically resolved via the CommandContext, allowing for more flexible evaluation.
• New Feature: List Slicing: Introduced ListSliceExpression to support list and string slicing (e.g., list[from..to]), including Python-style negative indices, enhancing Cypher's list manipulation capabilities.
• Improved Functionality and Robustness: Added support for the coalesce() function. The keys() and properties() functions now correctly handle null inputs and Map objects, with properties() preserving insertion order. The size() function now returns null for null or invalid inputs, aligning with Cypher's null propagation. Literal parsing has been improved to handle various number formats (e.g., .1, hex, octal) and detect integer overflows.
• Stricter Semantic Validation: Implemented new semantic validation rules for variable scope (especially for CALL with YIELD and SUBQUERY clauses) and for SKIP/LIMIT values, ensuring they are non-negative. Boolean operands are now strictly checked to prevent List or Map expressions from being used directly in boolean contexts.
• Refined Result Formatting: Adjusted the FinalProjectionStep to ensure that when a single document element is returned, it is correctly identified as an element for backward compatibility while retaining its original projection name for wire protocols. This resolves inconsistencies in how single-element results were presented.
• Traversal Logic Adjustment: Removed the IllegalArgumentException for minHops > maxHops in GraphTraverser, allowing such traversals to naturally yield empty results instead of throwing an error.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

🧪 CI Insights

Here's what we observed from your CI run for 25d73b7.

🟢 All jobs passed!

But CI Insights is watching 👀

OpenCypher TCK Compliance Improvements - Code Review

Thank you for this significant improvement to OpenCypher TCK compliance (48% pass rate). This PR implements several critical features for Cypher expression handling. Here's my detailed review:

✅ Strengths

1. List Slicing Implementation

The new ListSliceExpression class is well-designed:

• Properly handles negative indices (Python-style)
• Correctly clamps indices to valid ranges
• Supports both List and String slicing
• Returns appropriate empty values when from >= to
• Good null handling throughout

2. List Concatenation

The arithmetic expression enhancements for list operations are solid:

• Handles List + List (concatenation)
• Handles List + element (append)
• Handles element + List (prepend)
• Preserves existing string concatenation behavior

3. Function Resolution Architecture

The function resolver mechanism is a good architectural solution:

• Properly decouples function resolution from evaluation
• Stores resolver in context for compound expressions
• Allows function calls within AND/OR/CASE expressions

4. Semantic Validation Enhancements

Good additions:

• SKIP/LIMIT negative value validation
• Variable scope handling for CALL and SUBQUERY clauses
• Boolean operand type validation for lists/maps

⚠️ Issues & Concerns

1. Inconsistent Null Handling (High Priority)

Location: CypherFunctionFactory.java:381-389

The size() function behavior changed:

• Old: size(123) → 0L
• New: size(123) → null

Questions:

1. Is this change intentional and required by TCK?
2. Have you verified this doesn't break existing queries?
3. Should there be a migration note for users?

2. Expression Parser Complexity (Medium Priority)

Location: CypherExpressionBuilder.java:469-495

The addition of "top-level spine walk" methods adds significant complexity:

• These methods have similar structure with subtle differences
• The parsing order dependencies are fragile (comments indicate "MUST be checked BEFORE...")
• Hard to maintain and reason about

Recommendation: Consider extracting a common spine-walking utility method and add comprehensive unit tests specifically for expression parsing precedence.

3. Hex/Octal Number Parsing (Medium Priority)

Location: CypherExpressionBuilder.java:1767-1790

No handling for overflow in hex/octal parsing. Consider wrapping in try-catch:

try {
  final long val = Long.parseLong(hexPart, 16);
  return text.startsWith("-") ? -val : val;
} catch (NumberFormatException e) {
  throw new CommandParsingException("Invalid hex literal: " + text);
}

4. Exception Handling in Function Resolver (Low Priority)

Location: CypherExecutionPlan.java:271-278

Catching all exceptions and returning null silently swallows errors. Consider logging:

} catch (final Exception e) {
  context.getDatabase().getLogger().warn("Failed to resolve function: " + name, e);
  return null;
}

5. Code Style Consistency (Minor)

Location: CypherFunctionFactory.java:1398

Why change from List.of(args) to Arrays.asList(args)? Both create immutable lists, but List.of() is more modern.

🧪 Test Coverage

Recommend adding tests for:

1. List slicing edge cases:

   • [1,2,3][-10..10] (indices beyond bounds)
   • [1,2,3][5..2] (from > to)
   • "hello"[1..3] (string slicing)

2. List arithmetic:

   • [1,2] + [3,4] (concatenation)
   • [1,2] + 3 (append)
   • Chains: [1] + [2] + [3]

3. Number literal parsing:

   • 0x7FFFFFFFFFFFFFFF (max hex)
   • 0x8000000000000000 (overflow)
   • .5, -.5, .5e-3 (floats without integer part)

4. Negative SKIP/LIMIT validation:

   • RETURN 1 SKIP -1 (should fail)

📊 Performance

List concatenation creates new instances on every + operation. This is correct for immutability but consider documenting the copy-on-write semantics.

🔒 Security

No major security concerns identified. The changes handle null inputs safely and validate negative SKIP/LIMIT values appropriately.

✨ Recommendations Summary

High Priority:

1. Verify the size() null return change is intentional and document it
2. Add exception logging in function resolver
3. Add overflow handling for hex/octal parsing

Medium Priority:

4. Add unit tests for list slicing, arithmetic, and number parsing
5. Simplify expression parser spine-walking if possible

Low Priority:

6. Explain the Arrays.asList() change
7. Add JavaDoc for unchecked list casts

🎯 Overall Assessment

This is solid work that significantly improves OpenCypher TCK compliance. The code is generally well-structured and follows the project's coding standards. The main concerns are around:

1. Breaking behavioral changes (size() returning null)
2. Expression parser complexity
3. Test coverage for edge cases

Approval Status: ✅ Approve with minor changes recommended

Great progress on TCK compliance! 🚀

Coverage summary from Codacy

See diff coverage on Codacy

See your quality gate settings    Change summary preferences

Codecov Report

❌ Patch coverage is 32.04225% with 193 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.21%. Comparing base (44fdfa6) to head (25d73b7).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3362      +/-   ##
==========================================
+ Coverage   61.05%   61.21%   +0.15%     
==========================================
  Files        1165     1166       +1     
  Lines       78224    78483     +259     
  Branches    15412    15507      +95     
==========================================
+ Hits        47763    48041     +278     
+ Misses      23707    23593     -114     
- Partials     6754     6849      +95     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.