Skip to content

Commit a372b08

Browse files
Fix shift/reduce conflict in grammar (apache#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.
1 parent 852b48a commit a372b08

3 files changed

Lines changed: 103 additions & 32 deletions

File tree

regress/expected/list_comprehension.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,15 @@ SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2) WHERE
571571
ERROR: could not find rte for i
572572
LINE 1: ...$$ RETURN [i IN range(0, 10, 2) WHERE i>5 | i^2], i $$) AS (...
573573
^
574+
-- Invalid list comprehension
575+
SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) WHERE 2>5] $$) AS (result agtype);
576+
ERROR: Syntax error at or near IN
577+
LINE 1: SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN r...
578+
^
579+
SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) | 1] $$) AS (result agtype);
580+
ERROR: Syntax error at or near IN
581+
LINE 1: SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN r...
582+
^
574583
SELECT * FROM drop_graph('list_comprehension', true);
575584
NOTICE: drop cascades to 4 other objects
576585
DETAIL: drop cascades to table list_comprehension._ag_label_vertex

regress/sql/list_comprehension.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,8 @@ SELECT * FROM cypher('list_comprehension', $$ WITH [u IN [1,2,3]] AS u, [u IN [1
145145
SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2)],i $$) AS (result agtype, i agtype);
146146
SELECT * FROM cypher('list_comprehension', $$ RETURN [i IN range(0, 10, 2) WHERE i>5 | i^2], i $$) AS (result agtype, i agtype);
147147

148+
-- Invalid list comprehension
149+
SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) WHERE 2>5] $$) AS (result agtype);
150+
SELECT * FROM cypher('list_comprehension', $$ RETURN [1 IN range(0, 10, 2) | 1] $$) AS (result agtype);
151+
148152
SELECT * FROM drop_graph('list_comprehension', true);

src/backend/parser/cypher_gram.y

Lines changed: 90 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,6 @@
139139
/* common */
140140
%type <node> where_opt
141141

142-
/* list comprehension optional mapping expression */
143-
%type <node> mapping_expr_opt
144-
145142
/* pattern */
146143
%type <list> pattern simple_path_opt_parens simple_path
147144
%type <node> path anonymous_path
@@ -255,7 +252,12 @@ static Node *build_comparison_expression(Node *left_grammar_node,
255252
char *opr_name, int location);
256253

257254
// list_comprehension
258-
static Node *build_list_comprehension_node(char *var_name, Node *expr,
255+
static Node *verify_rule_as_list_comprehension(Node *expr, Node *expr2,
256+
Node *where, Node *mapping_expr,
257+
int var_loc, int expr_loc,
258+
int where_loc, int mapping_loc);
259+
260+
static Node *build_list_comprehension_node(ColumnRef *var_name, Node *expr,
259261
Node *where, Node *mapping_expr,
260262
int var_loc, int expr_loc,
261263
int where_loc,int mapping_loc);
@@ -2046,20 +2048,51 @@ list:
20462048

20472049
$$ = (Node *)n;
20482050
}
2049-
| '[' list_comprehension ']'
2050-
{
2051-
$$ = $2;
2052-
}
2051+
| list_comprehension
20532052
;
20542053

2055-
mapping_expr_opt:
2056-
/* empty */
2054+
/*
2055+
* This grammar rule is generic to some extent. It can
2056+
* evaluate to either IN operator or list comprehension.
2057+
* This avoids shift/reduce errors between the two rules.
2058+
*/
2059+
list_comprehension:
2060+
'[' expr IN expr ']'
20572061
{
2058-
$$ = NULL;
2062+
Node *n = $2;
2063+
Node *result = NULL;
2064+
2065+
/*
2066+
* If the first expr is a ColumnRef(variable), then the rule
2067+
* should evaluate as a list comprehension. Otherwise, it should
2068+
* evaluate as an IN operator.
2069+
*/
2070+
if (nodeTag(n) == T_ColumnRef)
2071+
{
2072+
ColumnRef *cref = (ColumnRef *)n;
2073+
result = build_list_comprehension_node(cref, $4, NULL, NULL,
2074+
@2, @4, 0, 0);
2075+
}
2076+
else
2077+
{
2078+
result = (Node *)makeSimpleA_Expr(AEXPR_IN, "=", n, $4, @3);
2079+
}
2080+
$$ = result;
20592081
}
2060-
| '|' expr
2082+
| '[' expr IN expr WHERE expr ']'
20612083
{
2062-
$$ = $2;
2084+
$$ = verify_rule_as_list_comprehension($2, $4, $6, NULL,
2085+
@2, @4, @6, 0);
2086+
}
2087+
| '[' expr IN expr '|' expr ']'
2088+
{
2089+
$$ = verify_rule_as_list_comprehension($2, $4, NULL, $6,
2090+
@2, @4, 0, @6);
2091+
}
2092+
| '[' expr IN expr WHERE expr '|' expr ']'
2093+
{
2094+
$$ = verify_rule_as_list_comprehension($2, $4, $6, $8,
2095+
@2, @4, @6, @8);
20632096
}
20642097
;
20652098

@@ -2124,14 +2157,6 @@ expr_case_default:
21242157
}
21252158
;
21262159

2127-
list_comprehension:
2128-
var_name IN expr where_opt mapping_expr_opt
2129-
{
2130-
$$ = build_list_comprehension_node($1, $3, $4, $5,
2131-
@1, @3, @4, @5);
2132-
}
2133-
;
2134-
21352160
expr_var:
21362161
var_name
21372162
{
@@ -3088,15 +3113,57 @@ static cypher_relationship *build_VLE_relation(List *left_arg,
30883113
return cr;
30893114
}
30903115

3116+
// Helper function to verify that the rule is a list comprehension
3117+
static Node *verify_rule_as_list_comprehension(Node *expr, Node *expr2,
3118+
Node *where, Node *mapping_expr,
3119+
int var_loc, int expr_loc,
3120+
int where_loc, int mapping_loc)
3121+
{
3122+
Node *result = NULL;
3123+
3124+
/*
3125+
* If the first expression is a ColumnRef, then we can build a
3126+
* list_comprehension node.
3127+
* Else its an invalid use of IN operator.
3128+
*/
3129+
if (nodeTag(expr) == T_ColumnRef)
3130+
{
3131+
ColumnRef *cref = (ColumnRef *)expr;
3132+
result = build_list_comprehension_node(cref, expr2, where,
3133+
mapping_expr, var_loc,
3134+
expr_loc, where_loc,
3135+
mapping_loc);
3136+
}
3137+
else
3138+
{
3139+
ereport(ERROR,
3140+
(errcode(ERRCODE_SYNTAX_ERROR),
3141+
errmsg("Syntax error at or near IN")));
3142+
}
3143+
return result;
3144+
}
3145+
30913146
/* helper function to build a list_comprehension grammar node */
3092-
static Node *build_list_comprehension_node(char *var_name, Node *expr,
3147+
static Node *build_list_comprehension_node(ColumnRef *cref, Node *expr,
30933148
Node *where, Node *mapping_expr,
30943149
int var_loc, int expr_loc,
30953150
int where_loc, int mapping_loc)
30963151
{
30973152
ResTarget *res = NULL;
30983153
cypher_unwind *unwind = NULL;
3099-
ColumnRef *cref = NULL;
3154+
char *var_name = NULL;
3155+
Value *val;
3156+
3157+
// Extract name from cref
3158+
val = linitial(cref->fields);
3159+
3160+
if (!IsA(val, String))
3161+
{
3162+
ereport(ERROR,
3163+
(errmsg_internal("unexpected Node for cypher_clause")));
3164+
}
3165+
3166+
var_name = val->val.str;
31003167

31013168
/*
31023169
* Build the ResTarget node for the UNWIND variable var_name attached to
@@ -3110,15 +3177,6 @@ static Node *build_list_comprehension_node(char *var_name, Node *expr,
31103177
/* build the UNWIND node */
31113178
unwind = make_ag_node(cypher_unwind);
31123179
unwind->target = res;
3113-
3114-
/*
3115-
* We need to make a ColumnRef of var_name so that it can be used as an expr
3116-
* for the where clause part of unwind.
3117-
*/
3118-
cref = makeNode(ColumnRef);
3119-
cref->fields = list_make1(makeString(var_name));
3120-
cref->location = var_loc;
3121-
31223180
unwind->where = where;
31233181

31243182
/* if there is a mapping function, add its arg to collect */

0 commit comments

Comments
 (0)