You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by de...@apache.org on 2022/10/26 21:40:59 UTC

[age] branch master updated: Invalid labels now return NULL

This is an automated email from the ASF dual-hosted git repository.

dehowef pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/age.git


The following commit(s) were added to refs/heads/master by this push:
     new 2fabc17  Invalid labels now return NULL
2fabc17 is described below

commit 2fabc17aa6be7846b21e0e574e9e2037d293eb2c
Author: Dehowe Feng <de...@gmail.com>
AuthorDate: Fri Oct 21 17:07:33 2022 -0700

    Invalid labels now return NULL
    
    Updated the behavior of invalid labels to return NULL rather than
    throw an error.
    
    Added additional regression tests as well.
---
 regress/expected/cypher_match.out  |  48 ++++++--
 regress/sql/cypher_match.sql       |   8 ++
 src/backend/parser/cypher_clause.c | 228 ++++++++++++++++++++++++++++++-------
 3 files changed, 228 insertions(+), 56 deletions(-)

diff --git a/regress/expected/cypher_match.out b/regress/expected/cypher_match.out
index 6327f10..4757392 100644
--- a/regress/expected/cypher_match.out
+++ b/regress/expected/cypher_match.out
@@ -523,21 +523,45 @@ LINE 2:  MATCH (a:v1)-[]-()-[]-(a {id:'will_fail'}) RETURN a
                                ^
 --Incorrect Labels
 SELECT * FROM cypher('cypher_match', $$MATCH (n)-[:v]-() RETURN n$$) AS (n agtype);
-ERROR:  label v is for vertices, not edges
-LINE 1: SELECT * FROM cypher('cypher_match', $$MATCH (n)-[:v]-() RET...
-                                                         ^
+ n 
+---
+(0 rows)
+
 SELECT * FROM cypher('cypher_match', $$MATCH (n)-[:emissing]-() RETURN n$$) AS (n agtype);
-ERROR:  label emissing does not exists
-LINE 1: SELECT * FROM cypher('cypher_match', $$MATCH (n)-[:emissing]...
-                                                         ^
+ n 
+---
+(0 rows)
+
 SELECT * FROM cypher('cypher_match', $$MATCH (n:e1)-[]-() RETURN n$$) AS (n agtype);
-ERROR:  label e1 is for edges, not vertices
-LINE 1: SELECT * FROM cypher('cypher_match', $$MATCH (n:e1)-[]-() RE...
-                                                     ^
+ n 
+---
+(0 rows)
+
 SELECT * FROM cypher('cypher_match', $$MATCH (n:vmissing)-[]-() RETURN n$$) AS (n agtype);
-ERROR:  label vmissing does not exists
-LINE 1: SELECT * FROM cypher('cypher_match', $$MATCH (n:vmissing)-[]...
-                                                     ^
+ n 
+---
+(0 rows)
+
+SELECT * FROM cypher('cypher_match', $$MATCH (:e1)-[r]-() RETURN r$$) AS (r agtype);
+ r 
+---
+(0 rows)
+
+SELECT * FROM cypher('cypher_match', $$MATCH (:vmissing)-[r]-() RETURN r$$) AS (r agtype);
+ r 
+---
+(0 rows)
+
+SELECT * FROM cypher('cypher_match', $$MATCH (n),(:e1) RETURN n$$) AS (n agtype);
+ n 
+---
+(0 rows)
+
+SELECT * FROM cypher('cypher_match', $$MATCH (n),()-[:v]-() RETURN n$$) AS (n agtype);
+ n 
+---
+(0 rows)
+
 --
 -- Path of one vertex. This should select 14
 --
diff --git a/regress/sql/cypher_match.sql b/regress/sql/cypher_match.sql
index 5d42d2e..1128296 100644
--- a/regress/sql/cypher_match.sql
+++ b/regress/sql/cypher_match.sql
@@ -293,6 +293,14 @@ SELECT * FROM cypher('cypher_match', $$MATCH (n:e1)-[]-() RETURN n$$) AS (n agty
 
 SELECT * FROM cypher('cypher_match', $$MATCH (n:vmissing)-[]-() RETURN n$$) AS (n agtype);
 
+SELECT * FROM cypher('cypher_match', $$MATCH (:e1)-[r]-() RETURN r$$) AS (r agtype);
+
+SELECT * FROM cypher('cypher_match', $$MATCH (:vmissing)-[r]-() RETURN r$$) AS (r agtype);
+
+SELECT * FROM cypher('cypher_match', $$MATCH (n),(:e1) RETURN n$$) AS (n agtype);
+
+SELECT * FROM cypher('cypher_match', $$MATCH (n),()-[:v]-() RETURN n$$) AS (n agtype);
+
 --
 -- Path of one vertex. This should select 14
 --
diff --git a/src/backend/parser/cypher_clause.c b/src/backend/parser/cypher_clause.c
index 70afb5f..8256287 100644
--- a/src/backend/parser/cypher_clause.c
+++ b/src/backend/parser/cypher_clause.c
@@ -128,10 +128,10 @@ static List *transform_match_path(cypher_parsestate *cpstate, Query *query,
                                   cypher_path *path);
 static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
                                    cypher_relationship *rel,
-                                   List **target_list);
+                                   List **target_list, bool valid_label);
 static Expr *transform_cypher_node(cypher_parsestate *cpstate,
                                    cypher_node *node, List **target_list,
-                                   bool output_node);
+                                   bool output_node, bool valid_label);
 static Node *make_vertex_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
                               char *label);
 static Node *make_edge_expr(cypher_parsestate *cpstate, RangeTblEntry *rte,
@@ -2139,6 +2139,64 @@ static Query *transform_cypher_with(cypher_parsestate *cpstate,
                                               wrapper);
 }
 
+static bool match_check_valid_label(cypher_match *match,
+                                    cypher_parsestate *cpstate)
+{
+    ListCell *cell1;
+    ListCell *cell2;
+    cypher_path *path;
+
+    foreach(cell1, match->pattern)
+    {
+        int i = 0;
+        path = (cypher_path*) lfirst(cell1);
+
+        foreach(cell2, path->path)
+        {
+            if (i % 2 == 0)
+            {
+                cypher_node *node = NULL;
+
+                node = lfirst(cell2);
+
+                if (node->label)
+                {
+                    label_cache_data *lcd =
+                        search_label_name_graph_cache(node->label,
+                                                      cpstate->graph_oid);
+
+                    if (lcd == NULL ||
+                        lcd->kind != LABEL_KIND_VERTEX)
+                    {
+                        return false;
+                    }
+                }
+            }
+            else
+            {
+                cypher_relationship *rel = NULL;
+
+                rel = lfirst(cell2);
+
+                if (rel->label)
+                {
+                    label_cache_data *lcd =
+                        search_label_name_graph_cache(rel->label,
+                                                      cpstate->graph_oid);
+
+                    if (lcd == NULL || lcd->kind != LABEL_KIND_EDGE)
+                    {
+                        return false;
+                    }
+                }
+            }
+            i++;
+        }
+    }
+
+    return true;
+}
+
 static Query *transform_cypher_clause_with_where(cypher_parsestate *cpstate,
                                                  transform_method transform,
                                                  cypher_clause *clause)
@@ -2150,7 +2208,6 @@ static Query *transform_cypher_clause_with_where(cypher_parsestate *cpstate,
     cypher_call *call_self;
     Node *where;
 
-
     if (is_ag_node(self, cypher_call))
     {
         call_self = (cypher_call*) clause->self;
@@ -2218,6 +2275,24 @@ static Query *transform_cypher_clause_with_where(cypher_parsestate *cpstate,
 static Query *transform_cypher_match(cypher_parsestate *cpstate,
                                      cypher_clause *clause)
 {
+    cypher_match *match_self = (cypher_match*) clause->self;
+
+    if(!match_check_valid_label(match_self, cpstate))
+    {
+        cypher_bool_const *l = make_ag_node(cypher_bool_const);
+        cypher_bool_const *r = make_ag_node(cypher_bool_const);
+
+        l->boolean = true;
+        l->location = -1;
+        r->boolean = false;
+        r->location = -1;
+
+        /*if the label is invalid, create a paradoxical where to get null*/
+        match_self->where = (Node *)makeSimpleA_Expr(AEXPR_OP, "=",
+                                                     (Node *)l,
+                                                     (Node *)r, -1);
+    }
+
     return transform_cypher_clause_with_where(
         cpstate, transform_cypher_match_pattern, clause);
 }
@@ -3556,6 +3631,56 @@ static bool isa_special_VLE_case(cypher_path *path)
     return false;
 }
 
+static bool path_check_valid_label(cypher_path *path,
+                                   cypher_parsestate *cpstate)
+{
+    ListCell *lc = NULL;
+    int i = 0;
+
+    foreach (lc, path->path)
+    {
+        if (i % 2 == 0)
+        {
+            cypher_node *node = NULL;
+
+            node = lfirst(lc);
+
+            if (node->label)
+            {
+                label_cache_data *lcd =
+                    search_label_name_graph_cache(node->label,
+                                                  cpstate->graph_oid);
+
+                if (lcd == NULL || lcd->kind != LABEL_KIND_VERTEX)
+                {
+                    return false;
+                }
+            }
+        }
+        else
+        {
+            cypher_relationship *rel = NULL;
+
+            rel = lfirst(lc);
+
+            if (rel->label)
+            {
+                label_cache_data *lcd =
+                    search_label_name_graph_cache(rel->label,
+                                                  cpstate->graph_oid);
+
+                if (lcd == NULL || lcd->kind != LABEL_KIND_EDGE)
+                {
+                    return false;
+                }
+            }
+        }
+        i++;
+    }
+
+    return true;
+}
+
 /*
  * Iterate through the path and construct all edges and necessary vertices
  */
@@ -3569,8 +3694,10 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
     bool node_declared_in_prev_clause = false;
     transform_entity *prev_entity = NULL;
     bool special_VLE_case = false;
+    bool valid_label = true;
 
     special_VLE_case = isa_special_VLE_case(path);
+    valid_label = path_check_valid_label(path, cpstate);
 
     /*
      * Iterate through every node in the path, construct the expr node
@@ -3618,7 +3745,7 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
 
             /* transform vertex */
             expr = transform_cypher_node(cpstate, node, &query->targetList,
-                                         output_node);
+                                         output_node, valid_label);
 
             entity = make_transform_entity(cpstate, ENT_VERTEX, (Node *)node,
                                            expr);
@@ -3677,7 +3804,8 @@ static List *transform_match_entities(cypher_parsestate *cpstate, Query *query,
                     }
                 }
 
-                expr = transform_cypher_edge(cpstate, rel, &query->targetList);
+                expr = transform_cypher_edge(cpstate, rel, &query->targetList,
+                                             valid_label);
 
                 entity = make_transform_entity(cpstate, ENT_EDGE, (Node *)rel,
                                                expr);
@@ -3964,7 +4092,8 @@ static Node *make_qual(cypher_parsestate *cpstate,
 
 static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
                                    cypher_relationship *rel,
-                                   List **target_list)
+                                   List **target_list,
+                                   bool valid_label)
 {
     ParseState *pstate = (ParseState *)cpstate;
     char *schema_name;
@@ -3980,31 +4109,20 @@ static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
     {
         rel->label = AG_DEFAULT_LABEL_EDGE;
     }
-    else
+    else if(!valid_label)
     {
         /*
          *  XXX: Need to determine proper rules, for when label does not exist
          *  or is for an edge. Maybe labels and edges should share names, like
          *  in openCypher. But these are stand in errors, to prevent
          *  segmentation faults, and other errors.
+         *
+         *  Update: Nonexistent and mismatched labels now return a NULL value to
+         *  prevent segmentation faults, and other errors. We can also consider 
+         *  if an all-purpose label would be useful.
          */
-        label_cache_data *lcd =
-            search_label_name_graph_cache(rel->label, cpstate->graph_oid);
-
-        if (lcd == NULL)
-        {
-            ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                            errmsg("label %s does not exists", rel->label),
-                            parser_errposition(pstate, rel->location)));
-        }
+        rel->label = NULL;
 
-        if (lcd->kind != LABEL_KIND_EDGE)
-        {
-            ereport(ERROR,
-                    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                     errmsg("label %s is for vertices, not edges", rel->label),
-                     parser_errposition(pstate, rel->location)));
-        }
     }
 
     if (rel->name != NULL)
@@ -4077,7 +4195,16 @@ static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
     }
 
     schema_name = get_graph_namespace_name(cpstate->graph_name);
-    rel_name = get_label_relation_name(rel->label, cpstate->graph_oid);
+
+    if (valid_label)
+    {
+        rel_name = get_label_relation_name(rel->label, cpstate->graph_oid);
+    }
+    else
+    {
+        rel_name = AG_DEFAULT_LABEL_EDGE;
+    }
+
     label_range_var = makeRangeVar(schema_name, rel_name, -1);
     alias = makeAlias(rel->name, NIL);
 
@@ -4091,7 +4218,14 @@ static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
 
     resno = pstate->p_next_resno++;
 
-    expr = (Expr *)make_edge_expr(cpstate, rte, rel->label);
+    if (valid_label)
+    {
+        expr = (Expr *)make_edge_expr(cpstate, rte, rel->label);
+    }
+    else
+    {
+        expr = (Expr*)makeNullConst(AGTYPEOID, -1, InvalidOid);
+    }
 
     if (rel->name)
     {
@@ -4104,7 +4238,7 @@ static Expr *transform_cypher_edge(cypher_parsestate *cpstate,
 
 static Expr *transform_cypher_node(cypher_parsestate *cpstate,
                                    cypher_node *node, List **target_list,
-                                   bool output_node)
+                                   bool output_node, bool valid_label)
 {
     ParseState *pstate = (ParseState *)cpstate;
     char *schema_name;
@@ -4120,30 +4254,20 @@ static Expr *transform_cypher_node(cypher_parsestate *cpstate,
     {
         node->label = AG_DEFAULT_LABEL_VERTEX;
     }
-    else
+    else if (!valid_label)
     {
         /*
          *  XXX: Need to determine proper rules, for when label does not exist
          *  or is for an edge. Maybe labels and edges should share names, like
          *  in openCypher. But these are stand in errors, to prevent
          *  segmentation faults, and other errors.
+         *
+         *  Update: Nonexistent and mismatched labels now return a NULL value to
+         *  prevent segmentation faults, and other errors. We can also consider 
+         *  if an all-purpose label would be useful.
          */
-        label_cache_data *lcd =
-            search_label_name_graph_cache(node->label, cpstate->graph_oid);
+        node->label = NULL;
 
-        if (lcd == NULL)
-        {
-            ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                            errmsg("label %s does not exists", node->label),
-                            parser_errposition(pstate, node->location)));
-        }
-        if (lcd->kind != LABEL_KIND_VERTEX)
-        {
-            ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                            errmsg("label %s is for edges, not vertices",
-                                   node->label),
-                            parser_errposition(pstate, node->location)));
-        }
     }
 
     if (!output_node)
@@ -4222,7 +4346,16 @@ static Expr *transform_cypher_node(cypher_parsestate *cpstate,
     }
 
     schema_name = get_graph_namespace_name(cpstate->graph_name);
-    rel_name = get_label_relation_name(node->label, cpstate->graph_oid);
+
+    if (valid_label)
+    {
+        rel_name = get_label_relation_name(node->label, cpstate->graph_oid);
+    }
+    else
+    {
+        rel_name = AG_DEFAULT_LABEL_VERTEX;
+    }
+
     label_range_var = makeRangeVar(schema_name, rel_name, -1);
     alias = makeAlias(node->name, NIL);
 
@@ -4236,7 +4369,14 @@ static Expr *transform_cypher_node(cypher_parsestate *cpstate,
 
     resno = pstate->p_next_resno++;
 
-    expr = (Expr *)make_vertex_expr(cpstate, rte, node->label);
+    if (valid_label)
+    {
+        expr = (Expr *)make_vertex_expr(cpstate, rte, node->label);
+    }
+    else
+    {
+        expr = (Expr*)makeNullConst(AGTYPEOID, -1, InvalidOid);
+    }
 
     /* make target entry and add it */
     te = makeTargetEntry(expr, resno, node->name, false);