Skip to content

Refactor rewrite star projection #3136

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.

Sign up for GitHub

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

Open
nafraf wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Refactor rewrite star projection #3136

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
57ad9dc
WIP: Refactoring AST_RewriteStarProjections()
nafraf Jul 5, 2023
1137b34
collect_call_projections(): Remove yield_count validation
nafraf Jul 5, 2023
b6620f5
Reorganize shared code
nafraf Jul 5, 2023
ced298c
WIP: _rewrite_call_subquery_star_projections()
nafraf Jul 6, 2023
6cd3c3f
Test CALL{} using CREATE, MERGE, UNWIND, CALL
nafraf Jul 6, 2023
9d47264
replace_clause(): remove unused var scope_start
nafraf Jul 7, 2023
5906b06
Fix intermediate WITH * including new variable
nafraf Jul 7, 2023
fa6756d
Add pointer casting
nafraf Jul 7, 2023
ee1b813
Merge branch 'master' into rewrite_star_projection
nafraf Jul 7, 2023
2b9b226
Remove changes of PR #3139
nafraf Jul 10, 2023
f2e5b19
_Validate_RETURN_Clause(): Check RETURN * without vars in scope
nafraf Jul 11, 2023
4b82445
Merge branch 'master' into rewrite_star_projection
nafraf Jul 11, 2023
4b29940
Check if vars of RETURN * are part of outer scope
nafraf Jul 11, 2023
a86f7d5
Remove raxClone()
nafraf Jul 11, 2023
37f02b4
Add test CALL { ... RETURN *,var}
nafraf Jul 12, 2023
bd20572
Use vctx->ignore_identifiers in _Validate_RETURN_Clause
nafraf Jul 12, 2023
66aee9f
Add collect_non_star_projections()
nafraf Jul 12, 2023
7a3470c
Remove unneedded case from collect_return_projections()
nafraf Jul 12, 2023
63e8b06
Merge branch 'master' into rewrite_star_projection
nafraf Jul 17, 2023
70d09a9
Remove unneeded cast
nafraf Jul 17, 2023
ba88672
Fix query having WITH * after UNION
nafraf Jul 17, 2023
a622b67
Fix collect projection in _rewrite_call_subquery_star_projections()
nafraf Jul 18, 2023
acf9d3f
Add missing replace_clause in AST_RewriteStarProjections()
nafraf Jul 18, 2023
6382406
Remove unneeded casting
nafraf Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 85 additions & 76 deletions src/ast/ast_rewrite_star_projections.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,12 @@ static void replace_clause
(
cypher_astnode_t *root, // ast root
cypher_astnode_t *clause, // clause being replaced
int scope_start, // beginning of scope
int scope_end, // ending of scope
rax *identifiers // bound vars
) {
ASSERT(identifiers != NULL);
cypher_astnode_type_t t = cypher_astnode_type(clause);

//--------------------------------------------------------------------------
// collect identifiers
//--------------------------------------------------------------------------
if(identifiers == NULL) {
identifiers = raxNew();
collect_aliases_in_scope(root, scope_start, scope_end, identifiers);
}

//--------------------------------------------------------------------------
// determine number of projections
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -99,32 +91,19 @@ static void replace_clause
//--------------------------------------------------------------------------
// handle no projections
//--------------------------------------------------------------------------

// e.g.
// MATCH () RETURN *
// MATCH () WITH * RETURN *
if(nprojections == 0) {
if(t == CYPHER_AST_RETURN) {
// error if this is a RETURN clause with no aliases
// e.g.
// MATCH () RETURN *
ErrorCtx_SetError("RETURN * is not allowed when there are no variables in scope");
raxFree(identifiers);
return;
} else {
// build an empty projection
// to make variable-less WITH clauses work:
// MATCH () WITH * CREATE ()
struct cypher_input_range range = { 0 };
// build a null node to project and an empty identifier as its alias
cypher_astnode_t *expression = cypher_ast_null(range);
cypher_astnode_t *identifier = cypher_ast_identifier("", 1, range);
cypher_astnode_t *children[2];
children[0] = expression;
children[1] = identifier;
projections[proj_idx++] = cypher_ast_projection(expression,
identifier, children, 2, range);
}
if(nprojections == 0 && t == CYPHER_AST_WITH) {
// build an empty projection
// to make variable-less WITH clauses work:
// MATCH () WITH * CREATE ()
struct cypher_input_range range = { 0 };
// build a null node to project and an empty identifier as its alias
cypher_astnode_t *expression = cypher_ast_null(range);
cypher_astnode_t *identifier = cypher_ast_identifier("", 1, range);
cypher_astnode_t *children[2];
children[0] = expression;
children[1] = identifier;
projections[proj_idx++] = cypher_ast_projection(expression,
identifier, children, 2, range);
}

//--------------------------------------------------------------------------
Expand All @@ -150,7 +129,6 @@ static void replace_clause
// update `nprojections` to actual number of projections
// value might be reduced due to duplicates
nprojections = proj_idx;
raxFree(identifiers);

// prepare arguments for new return clause node
bool distinct = false ;
Expand Down Expand Up @@ -224,9 +202,10 @@ static void replace_clause
static bool _rewrite_call_subquery_star_projections
(
const cypher_astnode_t *wrapping_clause, // wrapping clause
uint scope_start, // start scope in wrapping clause
uint idx // index of the call subquery
uint idx, // index of the call subquery
rax *identifiers // bound vars
) {
rax *local_identifiers = raxNew();
bool rewritten = false;

// get the call subquery clause
Expand All @@ -238,7 +217,6 @@ static bool _rewrite_call_subquery_star_projections

Copy link
Copy Markdown
Collaborator

@raz-mon raz-mon Jul 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for casting here (can also remove the new-line).

Copy link
Copy Markdown
Contributor Author

@nafraf nafraf Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Casting removed

uint n_clauses = cypher_ast_query_nclauses(query);

uint first_in_scope = 0;
// initialize `last_is_union` to true to rewrite the first importing `WITH`
bool last_is_union = true;
for(uint i = 0; i < n_clauses; i++) {
Expand All @@ -250,7 +228,6 @@ static bool _rewrite_call_subquery_star_projections
cypher_astnode_t *inner_query =
cypher_ast_call_subquery_get_query(call_subquery);
rewritten |= AST_RewriteStarProjections(inner_query);
last_is_union = false;
} else if(t == CYPHER_AST_WITH || t == CYPHER_AST_RETURN) {
// check whether the clause contains a star projection
bool has_star = (t == CYPHER_AST_WITH) ?
Expand All @@ -264,29 +241,51 @@ static bool _rewrite_call_subquery_star_projections
if(has_star) {
if(last_is_union && t == CYPHER_AST_WITH) {
// importing `WITH` clause, import vars from wrapping clause
rax *identifiers = raxNew();
if(scope_start != idx) {
collect_aliases_in_scope(wrapping_clause,
scope_start, idx, identifiers);
}
replace_clause(query, clause, first_in_scope, i,
identifiers);
rax *clone_identifiers = raxClone(identifiers);
replace_clause(query, clause, i, clone_identifiers);
clause = (cypher_astnode_t *)
cypher_ast_query_get_clause(query, i);
raxFree(clone_identifiers);
} else {
// intermediate `WITH` or `RETURN` clause
replace_clause(query, clause, first_in_scope, i, NULL);
if(t == CYPHER_AST_WITH) {
collect_with_projections(clause, local_identifiers);
} else {
collect_return_projections(clause, local_identifiers);
}
replace_clause(query, clause, i, local_identifiers);
clause = (cypher_astnode_t *)
cypher_ast_query_get_clause(query, i);
}
rewritten = true;
} else {
// if the clause does not contain a star projection,
// the new scope only should contain the identifiers
// projected by the current clause.
// Example: Second WITH inside CALL {} in the following query
// WITH 1 AS a CALL { WITH * WITH a AS c RETURN *} RETURN *
raxFree(local_identifiers);
local_identifiers = raxNew();
}
first_in_scope = i;
last_is_union = false;
} else if(t == CYPHER_AST_UNION) {
first_in_scope = i + 1;

if(t == CYPHER_AST_WITH) {
collect_with_projections(clause, local_identifiers);
} else {
collect_return_projections(clause, local_identifiers);
}
Copy link
Copy Markdown
Collaborator

@raz-mon raz-mon Jul 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We enter these collection functions for a second time if we have a * (first in 252, 254).

Copy link
Copy Markdown
Contributor Author

@nafraf nafraf Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Please check the changes done to the function _rewrite_call_subquery_star_projections(), I think now it is simpler.

} else {
collect_non_star_projections(clause, local_identifiers);
}

// update last_is_union
if(t == CYPHER_AST_UNION) {
last_is_union = true;
} else {
last_is_union = false;
}
}

raxFree(local_identifiers);
return rewritten;
}

Expand All @@ -301,38 +300,48 @@ bool AST_RewriteStarProjections
}

// rewrite all WITH * / RETURN * clauses to include all aliases
uint scope_start = 0;
uint clause_count = cypher_ast_query_nclauses(root);
rax *identifiers = raxNew();

for(uint i = 0; i < clause_count; i ++) {
const cypher_astnode_t *clause = cypher_ast_query_get_clause(root, i);
cypher_astnode_type_t t = cypher_astnode_type(clause);
cypher_astnode_type_t type = cypher_astnode_type(clause);

if(t == CYPHER_AST_CALL_SUBQUERY) {
if(type == CYPHER_AST_CALL_SUBQUERY) {
// rewrite the CALL{} before collecting the identifiers
rewritten |=
_rewrite_call_subquery_star_projections(root, scope_start, i);
continue;
}

if(t != CYPHER_AST_WITH && t != CYPHER_AST_RETURN) {
continue;
}

bool has_include_existing = (t == CYPHER_AST_WITH) ?
cypher_ast_with_has_include_existing(clause) :
cypher_ast_return_has_include_existing(clause);

if(has_include_existing) {
// clause contains a star projection, replace it
replace_clause((cypher_astnode_t *)root, (cypher_astnode_t *)clause,
scope_start, i, NULL);
rewritten = true;
_rewrite_call_subquery_star_projections(root, i, identifiers);
clause = cypher_ast_query_get_clause(root, i);
collect_call_subquery_projections(clause, identifiers);
} else if(type == CYPHER_AST_WITH) {
if(cypher_ast_with_has_include_existing(clause)) {
// clause contains a star projection, replace it
replace_clause((cypher_astnode_t *)root,
(cypher_astnode_t *)clause, i, identifiers);
clause = cypher_ast_query_get_clause(root, i);
rewritten = true;
}
// update new scope identifiers
raxFree(identifiers);
identifiers = raxNew();
collect_with_projections(clause, identifiers);
} else if(type == CYPHER_AST_RETURN) {
if(cypher_ast_return_has_include_existing(clause)) {
// clause contains a star projection, replace it
replace_clause((cypher_astnode_t *)root,
(cypher_astnode_t *)clause, i, identifiers);
clause = cypher_ast_query_get_clause(root, i);
rewritten = true;
}
// update new scope identifiers
raxFree(identifiers);
identifiers = raxNew();
collect_return_projections(clause, identifiers);
} else {
collect_non_star_projections(clause, identifiers);
}

// update scope start
scope_start = i;
}

raxFree(identifiers);
return rewritten;
}

Loading