Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3107 +/- ##
==========================================
- Coverage 90.87% 90.87% -0.01%
==========================================
Files 294 294
Lines 30269 30257 -12
==========================================
- Hits 27508 27495 -13
- Misses 2761 2762 +1
☔ View full report in Codecov by Sentry. |
raz-mon
left a comment
There was a problem hiding this comment.
Nice work!
See few comments.
| "WITH 1 AS a CALL {WITH a SKIP 5 RETURN a} RETURN a", | ||
| "WITH true AS a CALL {WITH NOT(a) AS b RETURN b} RETURN b", | ||
| "MATCH (n) CALL {WITH n AS n1 RETURN n1 UNION WITH n RETURN n1} RETURN n, n1", | ||
| "MATCH (n) CALL {WITH n RETURN n AS n1 UNION WITH n AS n1 RETURN n1} RETURN n, n1", |
There was a problem hiding this comment.
what's the issue with this last query ?
There was a problem hiding this comment.
Non-simple import in the second branch of the UNION.
| return aggregated; | ||
| } | ||
|
|
||
| const char *AST_GetProjectionAlias |
There was a problem hiding this comment.
Please copy the comments from the header file to this file.
| const char *alias = AST_GetProjectionAlias(projection); | ||
| if(alias != NULL) { | ||
| array_append(columns, alias); | ||
| } |
There was a problem hiding this comment.
Seems like AST_GetProjectionAlias can't return NULL if my understanding is correct please remove this condition.
| if(strcmp(projections[j], alias) != 0) { | ||
| const cypher_astnode_t *proj = | ||
| cypher_ast_return_get_projection(return_clause, j); | ||
| const char *alias = AST_GetProjectionAlias(proj); |
There was a problem hiding this comment.
is it possible for AST_GetProjectionAlias to return NULL ?
There was a problem hiding this comment.
No, it can't return NULL. The unneeded comparison was removed from if(alias == NULL || ...
| cypher_ast_return_get_projection(return_clause, i); | ||
| const char *var_name; | ||
| const cypher_astnode_t *identifier = | ||
| const cypher_astnode_t *alias_node = |
There was a problem hiding this comment.
No, we don't need it. The variable was removed in 23fa13d
Thanks.
| if(alias_node == NULL) { | ||
| continue; | ||
| const cypher_astnode_t *proj = cypher_ast_return_get_projection(n, i); | ||
| const char *alias = AST_GetProjectionAlias(proj); |
There was a problem hiding this comment.
In which case AST_GetProjectionAlias returns NULL ?
There was a problem hiding this comment.
It can no longer return NULL, the condition is no longer needed.
There was a problem hiding this comment.
OK. The condition was removed.
| // returns the alias of a projection | ||
| // the alias will be NULL in the case of an unaliased non-identifier projection | ||
| // returned in a Call {} clause | ||
| const char *AST_GetSubqueryProjectionAlias |
There was a problem hiding this comment.
Couldn't find AST_GetSubqueryProjectionAlias implementation.
There was a problem hiding this comment.
We don't need this function. Its definition was deleted in 23fa13d. Thanks
Patch to validate that the returning literals in CALL subquery are aliased.