Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement list comprehension #1736

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Conversation

MuhammadTahaNaveed
Copy link
Member

Adds the OpenCypher spec for List Comprehension to AGE. Also added a commit to remove shift/reduce conflict in grammar.

Syntax

[
     <variable>   // variable to assume values from list
     IN <list>      // a list or something that creates one
     WHERE <condition>    // optional condition using the above variable
     | <mapping>     // optional mapping function
]

Example

[u IN [1,2,3] WHERE u>1 | u^2]

Known Issue

List comprehension as property constraint of MERGE clause errors out. Created issue #1611 for that.

Co-authored-by:
John Jemignani
Zainab Saad
Muhammad Taha Naveed

MuhammadTahaNaveed and others added 2 commits April 8, 2024 10:22
* Add initial code for list comprehension

- Additionally, fixed a bug in nested queries.

* Resolve variable scope and ambigous variable issue

- For scoping the variable, remove the pnsi from namespace list after
  being used by the list comprehension.

- To resolve the ambigous variable issue, only scan the pnsi of
  list_comp subquery.

* Fix non agg plan node issue in list comprehension

- Resolved the issue 'aggref found in non-agg plan node'
  when the list comprehension is used as a property constraint in
  MATCH and other clauses or when used in WHERE clause. Intead of adding
  qual in the jointree, we are now adding the qual in having node of
  query.

  e.g.
  MATCH (n {list: [for i in [1,2,3]]})
  MATCH (n) WHERE n.list=[for i in [1,2,3]]
  WITH n WHERE n.list=[for i in [1,2,3]]

- Removed redundant call to function tranform_cypher_clause_with_where
  by tranform_match_clause.

* Resolve variable reuse issue in list comprehension

- Also, for scoping list comprehension variable, instead of removing
  the pnsi of list comp from namespace, now we just mark the cols of
  pnsi as not visible.

* Modify grammer to match opencypher list comprehension grammer

- Also added initial regression tests for list comprehension

* Add regression tests for bugs resolved

- Includes tests for nested cases, ambigous variable issue, list
  comprehension variable scoping and planner node aggref issue.

- Should be added more.

* Resolve invalid output on multiple list comprehensions

- Fix the output of multiple list comprehensions in the RETURN clause
  and the WHERE clause.
- Added regression tests for the above and some for the previous bugs
  resolved.

* Fix aggref found where not expected issue (#189)

- Resolved the 'aggref found where not expected' error thrown when
  using list comprehensions in UNWIND, or as a property update
  expression in SET or in the comparison expressions, typecasts or
  string matching in WITH - WHERE clause

- Added an error to be thrown when using aggregation functions in UNWIND
  and property update expr in SET clause

- Added regression tests

* cleanup: Remove dead code and formatting changes

---------

Co-authored-by: John Gemignani <[email protected]>
Co-authored-by: Zainab Saad <[email protected]>
- The grammar had a shift/reduce conflict due to the ambiguity of the
  `IN` keyword. This is resolved by adding generic rule and manually
  resolving to the correct specific rule.
- Added additional test cases.
@github-actions github-actions bot added the PG16 label Apr 8, 2024
@dehowef
Copy link
Member

dehowef commented Apr 9, 2024

Looks good to me~

@dehowef dehowef merged commit c967c64 into apache:PG16 Apr 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants