Skip to content

Conversation

@jrgemignani
Copy link
Contributor

@jrgemignani jrgemignani commented Dec 10, 2025

Consolidated duplicate code, added helper functions, and reviewed the grammar file for issues.

NOTE: I used an AI tool to review and cleanup the grammar file. I
have reviewed all of the work it did.

Improvements:

  1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths
  2. Consolidated EXPLAIN statement handling into make_explain_stmt helper
  3. Extracted WITH clause validation into validate_return_item_aliases helper
  4. Created make_default_return_node helper for subquery return-less logic

Benefits:

  • Reduced code duplication by ~150 lines
  • Improved maintainability with helper functions
  • Eliminated manual string length calculations (error-prone)

All 29 existing regression tests pass

modified: src/backend/parser/cypher_gram.y

@github-actions github-actions bot added master override-stale To keep issues/PRs untouched from stale action labels Dec 10, 2025
@jrgemignani jrgemignani force-pushed the cleanup_grammar_file branch 2 times, most recently from d428791 to a90c7de Compare December 10, 2025 17:38
@MuhammadTahaNaveed
Copy link
Member

@jrgemignani I don't think there's a need for adding a separate test file just for these tests. These tests are already in abundance in other test files. Maybe in future we should have a separate one for grammar, but it should be a comprehensive test suite and should cover all constructs.

Consolidated duplicate code, added helper functions, and reviewed
the grammar file for issues.

NOTE: I used an AI tool to review and cleanup the grammar file. I
      have reviewed all of the work it did.

Improvements:

1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths
2. Consolidated EXPLAIN statement handling into make_explain_stmt helper
3. Extracted WITH clause validation into validate_return_item_aliases helper
4. Created make_default_return_node helper for subquery return-less logic

Benefits:

- Reduced code duplication by ~150 lines
- Improved maintainability with helper functions
- Eliminated manual string length calculations (error-prone)

All 29 existing regression tests pass

modified:   src/backend/parser/cypher_gram.y
@jrgemignani
Copy link
Contributor Author

@jrgemignani I don't think there's a need for adding a separate test file just for these tests. These tests are already in abundance in other test files. Maybe in future we should have a separate one for grammar, but it should be a comprehensive test suite and should cover all constructs.

removed.

@MuhammadTahaNaveed MuhammadTahaNaveed merged commit 91de779 into apache:master Dec 11, 2025
7 checks passed
jrgemignani added a commit to jrgemignani/age that referenced this pull request Dec 16, 2025
Consolidated duplicate code, added helper functions, and reviewed
the grammar file for issues.

NOTE: I used an AI tool to review and cleanup the grammar file. I
      have reviewed all of the work it did.

Improvements:

1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths
2. Consolidated EXPLAIN statement handling into make_explain_stmt helper
3. Extracted WITH clause validation into validate_return_item_aliases helper
4. Created make_default_return_node helper for subquery return-less logic

Benefits:

- Reduced code duplication by ~150 lines
- Improved maintainability with helper functions
- Eliminated manual string length calculations (error-prone)

All 29 existing regression tests pass

modified:   src/backend/parser/cypher_gram.y
MuhammadTahaNaveed pushed a commit that referenced this pull request Dec 16, 2025
Consolidated duplicate code, added helper functions, and reviewed
the grammar file for issues.

NOTE: I used an AI tool to review and cleanup the grammar file. I
      have reviewed all of the work it did.

Improvements:

1. Added KEYWORD_STRDUP macro to eliminate hardcoded string lengths
2. Consolidated EXPLAIN statement handling into make_explain_stmt helper
3. Extracted WITH clause validation into validate_return_item_aliases helper
4. Created make_default_return_node helper for subquery return-less logic

Benefits:

- Reduced code duplication by ~150 lines
- Improved maintainability with helper functions
- Eliminated manual string length calculations (error-prone)

All 29 existing regression tests pass

modified:   src/backend/parser/cypher_gram.y
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

master override-stale To keep issues/PRs untouched from stale action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants