From fab3119a109280fd63237ce17c6d4dd60b7dfc03 Mon Sep 17 00:00:00 2001 From: Muhammad Taha Naveed Date: Fri, 5 Apr 2024 04:36:25 +0500 Subject: [PATCH] Fix shift/reduce conflict in grammar (#1719) - 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. --- regress/expected/list_comprehension.out | 9 ++ regress/sql/list_comprehension.sql | 4 + src/backend/parser/cypher_gram.y | 122 +++++++++++++++++------- 3 files changed, 103 insertions(+), 32 deletions(-) diff --git a/regress/expected/list_comprehension.out b/regress/expected/list_comprehension.out index 8b7a64531..5b1721187 100644 --- a/regress/expected/list_comprehension.out +++ b/regress/expected/list_comprehension.out @@ -571,6 +571,15 @@ SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2) WHERE ERROR: could not find rte for i LINE 1: ...$$ RETURN [i IN range(0, 10, 2) WHERE i>5 | i^2], i $$) AS (... ^ +-- Invalid list comprehension +SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) WHERE 2>5] $$) AS (result agtype); +ERROR: Syntax error at or near IN +LINE 1: SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN r... + ^ +SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) | 1] $$) AS (result agtype); +ERROR: Syntax error at or near IN +LINE 1: SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN r... + ^ SELECT * FROM drop_graph('list_comprehension', true); NOTICE: drop cascades to 4 other objects DETAIL: drop cascades to table list_comprehension._ag_label_vertex diff --git a/regress/sql/list_comprehension.sql b/regress/sql/list_comprehension.sql index b4596e55c..6a33b6497 100644 --- a/regress/sql/list_comprehension.sql +++ b/regress/sql/list_comprehension.sql @@ -145,4 +145,8 @@ SELECT * FROM cypher('list_comprehension', $$ WITH [u IN [1,2,3]] AS u, [u IN [1 SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2)],i $$) AS (result agtype, i agtype); SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2) WHERE i>5 | i^2], i $$) AS (result agtype, i agtype); +-- Invalid list comprehension +SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) WHERE 2>5] $$) AS (result agtype); +SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) | 1] $$) AS (result agtype); + SELECT * FROM drop_graph('list_comprehension', true); \ No newline at end of file diff --git a/src/backend/parser/cypher_gram.y b/src/backend/parser/cypher_gram.y index a1878afe7..359c3a85d 100644 --- a/src/backend/parser/cypher_gram.y +++ b/src/backend/parser/cypher_gram.y @@ -139,9 +139,6 @@ /* common */ %type where_opt -/* list comprehension optional mapping expression */ -%type mapping_expr_opt - /* pattern */ %type pattern simple_path_opt_parens simple_path %type path anonymous_path @@ -258,7 +255,12 @@ static Node *build_comparison_expression(Node *left_grammar_node, char *opr_name, int location); // list_comprehension -static Node *build_list_comprehension_node(char *var_name, Node *expr, +static Node *verify_rule_as_list_comprehension(Node *expr, Node *expr2, + Node *where, Node *mapping_expr, + int var_loc, int expr_loc, + int where_loc, int mapping_loc); + +static Node *build_list_comprehension_node(ColumnRef *var_name, Node *expr, Node *where, Node *mapping_expr, int var_loc, int expr_loc, int where_loc,int mapping_loc); @@ -2128,20 +2130,51 @@ list: $$ = (Node *)n; } - | '[' list_comprehension ']' - { - $$ = $2; - } + | list_comprehension ; -mapping_expr_opt: - /* empty */ +/* + * This grammar rule is generic to some extent. It can + * evaluate to either IN operator or list comprehension. + * This avoids shift/reduce errors between the two rules. + */ +list_comprehension: + '[' expr IN expr ']' { - $$ = NULL; + Node *n = $2; + Node *result = NULL; + + /* + * If the first expr is a ColumnRef(variable), then the rule + * should evaluate as a list comprehension. Otherwise, it should + * evaluate as an IN operator. + */ + if (nodeTag(n) == T_ColumnRef) + { + ColumnRef *cref = (ColumnRef *)n; + result = build_list_comprehension_node(cref, $4, NULL, NULL, + @2, @4, 0, 0); + } + else + { + result = (Node *)makeSimpleA_Expr(AEXPR_IN, "=", n, $4, @3); + } + $$ = result; } - | '|' expr + | '[' expr IN expr WHERE expr ']' { - $$ = $2; + $$ = verify_rule_as_list_comprehension($2, $4, $6, NULL, + @2, @4, @6, 0); + } + | '[' expr IN expr '|' expr ']' + { + $$ = verify_rule_as_list_comprehension($2, $4, NULL, $6, + @2, @4, 0, @6); + } + | '[' expr IN expr WHERE expr '|' expr ']' + { + $$ = verify_rule_as_list_comprehension($2, $4, $6, $8, + @2, @4, @6, @8); } ; @@ -2206,14 +2239,6 @@ expr_case_default: } ; -list_comprehension: - var_name IN expr where_opt mapping_expr_opt - { - $$ = build_list_comprehension_node($1, $3, $4, $5, - @1, @3, @4, @5); - } -; - expr_var: var_name { @@ -3179,15 +3204,57 @@ static cypher_relationship *build_VLE_relation(List *left_arg, return cr; } +// Helper function to verify that the rule is a list comprehension +static Node *verify_rule_as_list_comprehension(Node *expr, Node *expr2, + Node *where, Node *mapping_expr, + int var_loc, int expr_loc, + int where_loc, int mapping_loc) +{ + Node *result = NULL; + + /* + * If the first expression is a ColumnRef, then we can build a + * list_comprehension node. + * Else its an invalid use of IN operator. + */ + if (nodeTag(expr) == T_ColumnRef) + { + ColumnRef *cref = (ColumnRef *)expr; + result = build_list_comprehension_node(cref, expr2, where, + mapping_expr, var_loc, + expr_loc, where_loc, + mapping_loc); + } + else + { + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("Syntax error at or near IN"))); + } + return result; +} + /* helper function to build a list_comprehension grammar node */ -static Node *build_list_comprehension_node(char *var_name, Node *expr, +static Node *build_list_comprehension_node(ColumnRef *cref, Node *expr, Node *where, Node *mapping_expr, int var_loc, int expr_loc, int where_loc, int mapping_loc) { ResTarget *res = NULL; cypher_unwind *unwind = NULL; - ColumnRef *cref = NULL; + char *var_name = NULL; + String *val; + + // Extract name from cref + val = linitial(cref->fields); + + if (!IsA(val, String)) + { + ereport(ERROR, + (errmsg_internal("unexpected Node for cypher_clause"))); + } + + var_name = val->sval; /* * Build the ResTarget node for the UNWIND variable var_name attached to @@ -3201,15 +3268,6 @@ static Node *build_list_comprehension_node(char *var_name, Node *expr, /* build the UNWIND node */ unwind = make_ag_node(cypher_unwind); unwind->target = res; - - /* - * We need to make a ColumnRef of var_name so that it can be used as an expr - * for the where clause part of unwind. - */ - cref = makeNode(ColumnRef); - cref->fields = list_make1(makeString(var_name)); - cref->location = var_loc; - unwind->where = where; /* if there is a mapping function, add its arg to collect */