Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3136 +/- ##
==========================================
+ Coverage 90.85% 90.86% +0.01%
==========================================
Files 294 294
Lines 30269 30304 +35
==========================================
+ Hits 27502 27537 +35
Misses 2767 2767
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice work, see some comments.
| @@ -238,7 +217,6 @@ static bool _rewrite_call_subquery_star_projections | |||
|
|
|||
There was a problem hiding this comment.
No need for casting here (can also remove the new-line).
| # assert results | ||
| self.env.assertEquals(len(res.result_set), 1) | ||
| self.env.assertEquals(res.result_set[0], [1, 2]) | ||
|
|
There was a problem hiding this comment.
The following case fails:
GRAPH.QUERY g "WITH 1 AS one CALL {RETURN one AS num UNION WITH * WITH * WITH one AS num} RETURN num"I think the separation between the identifiers and the local_identifiers in _rewrite_call_subquery_star_projections() is probably necessary, but it needs some work.
There was a problem hiding this comment.
OK. New tests were added. Now we don't collect identifiers for the RETURN clauses in _rewrite_call_subquery_star_projections(), the identifiers are collected later.
| if(t == CYPHER_AST_WITH) { | ||
| collect_with_projections(clause, local_identifiers); | ||
| } else { | ||
| collect_return_projections(clause, local_identifiers); | ||
| } |
There was a problem hiding this comment.
We enter these collection functions for a second time if we have a * (first in 252, 254).
There was a problem hiding this comment.
OK. Please check the changes done to the function _rewrite_call_subquery_star_projections(), I think now it is simpler.
Refactor
AST_RewriteStarProjections()and_rewrite_call_subquery_star_projections()