MuhammadTahaNaveed commented on code in PR #2559:
URL: https://github.com/apache/age/pull/2559#discussion_r4251546504


##########
src/backend/parser/cypher_clause.c:
##########
@@ -4621,6 +4629,132 @@ static ParseNamespaceItem 
*transform_RangeFunction(cypher_parsestate *cpstate,
     return pnsi;
 }
 
+static const char *find_static_edge_label(List *pattern,
+                                          const char *variable_name)
+{
+    ListCell *path_cell;
+    const char *label = NULL;
+
+    foreach (path_cell, pattern)
+    {
+        cypher_path *path = lfirst(path_cell);
+        ListCell *entity_cell;
+
+        foreach (entity_cell, path->path)
+        {
+            Node *entity = lfirst(entity_cell);
+            cypher_relationship *rel;
+
+            if (!is_ag_node(entity, cypher_relationship))
+                continue;
+
+            rel = (cypher_relationship *)entity;
+            if (rel->name == NULL ||
+                strcmp(rel->name, variable_name) != 0)
+                continue;
+
+            if (rel->varlen != NULL || rel->parsed_label == NULL)

Review Comment:
   Please exclude `IS_DEFAULT_LABEL_EDGE(rel->parsed_label)` here. 
`_ag_label_edge` scans child labels, so this predicate isn't redundant. This 
returns `"P"` on the PR and no rows on master:
   
   ```sql
   CREATE EXTENSION IF NOT EXISTS age;
   LOAD 'age';
   SET search_path = ag_catalog, public;
   
   SELECT create_graph('type_filter_test');
   SELECT * FROM cypher('type_filter_test', $$
       CREATE ()-[:P]->()
   $$) AS (v agtype);
   
   SELECT * FROM cypher('type_filter_test', $$
       MATCH ()-[r:_ag_label_edge]->()
       WHERE type(r) = '_ag_label_edge'
       RETURN type(r)
   $$) AS (edge_type agtype);
   ```
   
   Please cover this case in `regress/sql/expr.sql`.



##########
src/backend/parser/cypher_clause.c:
##########
@@ -4621,6 +4629,132 @@ static ParseNamespaceItem 
*transform_RangeFunction(cypher_parsestate *cpstate,
     return pnsi;
 }
 
+static const char *find_static_edge_label(List *pattern,
+                                          const char *variable_name)
+{
+    ListCell *path_cell;
+    const char *label = NULL;
+
+    foreach (path_cell, pattern)
+    {
+        cypher_path *path = lfirst(path_cell);
+        ListCell *entity_cell;
+
+        foreach (entity_cell, path->path)
+        {
+            Node *entity = lfirst(entity_cell);
+            cypher_relationship *rel;
+
+            if (!is_ag_node(entity, cypher_relationship))
+                continue;
+
+            rel = (cypher_relationship *)entity;
+            if (rel->name == NULL ||
+                strcmp(rel->name, variable_name) != 0)
+                continue;
+
+            if (rel->varlen != NULL || rel->parsed_label == NULL)
+                return NULL;
+
+            if (label != NULL && strcmp(label, rel->parsed_label) != 0)
+                return NULL;
+
+            label = rel->parsed_label;
+        }
+    }
+
+    return label;
+}
+
+static bool is_redundant_edge_type_qual(List *pattern, Node *qual)
+{
+    cypher_comparison_aexpr *comparison;
+    FuncCall *function;
+    ColumnRef *column;
+    A_Const *constant;
+    String *operator_name;
+    String *function_name;
+    String *variable_name;
+    const char *static_label;
+
+    if (qual == NULL || !is_ag_node(qual, cypher_comparison_aexpr))
+        return false;
+
+    comparison = (cypher_comparison_aexpr *)qual;
+    if (comparison->kind != AEXPR_OP ||
+        list_length(comparison->name) != 1)
+        return false;
+
+    operator_name = linitial(comparison->name);
+    if (!IsA(operator_name, String) ||
+        strcmp(strVal(operator_name), "=") != 0 ||
+        !IsA(comparison->lexpr, FuncCall) ||
+        !IsA(comparison->rexpr, A_Const))
+        return false;
+
+    function = (FuncCall *)comparison->lexpr;
+    constant = (A_Const *)comparison->rexpr;
+    if (list_length(function->funcname) != 1 ||
+        list_length(function->args) != 1 ||
+        constant->isnull ||
+        constant->val.sval.type != T_String)
+        return false;
+
+    function_name = linitial(function->funcname);
+    if (!IsA(function_name, String) ||
+        pg_strcasecmp(strVal(function_name), "type") != 0 ||
+        !IsA(linitial(function->args), ColumnRef))
+        return false;
+
+    column = linitial(function->args);
+    if (list_length(column->fields) != 1 ||
+        !IsA(linitial(column->fields), String))
+        return false;
+
+    variable_name = linitial(column->fields);
+    static_label = find_static_edge_label(pattern, strVal(variable_name));
+
+    return static_label != NULL &&
+           strcmp(static_label, strVal(&constant->val)) == 0;
+}
+
+static Node *simplify_redundant_edge_type_qual(List *pattern, Node *where)
+{
+    BoolExpr *expression;
+    List *args = NIL;
+    ListCell *cell;
+
+    if (where == NULL)
+        return NULL;
+
+    if (is_redundant_edge_type_qual(pattern, where))
+        return NULL;
+
+    if (!IsA(where, BoolExpr))
+        return where;
+
+    expression = (BoolExpr *)where;
+    if (expression->boolop != AND_EXPR)
+        return where;
+
+    foreach (cell, expression->args)
+    {
+        Node *arg = lfirst(cell);
+
+        if (!is_redundant_edge_type_qual(pattern, arg))

Review Comment:
   Non-blocking: could this simplify nested `AND` expressions too? `WHERE r.v > 
0 AND (type(r) = 'P' AND r.v < 4)` keeps the type filter, while the equivalent 
flat expression removes it. Reusing this helper recursively would make 
parentheses behave consistently.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to