Copilot commented on code in PR #2468:
URL: https://github.com/apache/age/pull/2468#discussion_r3562099251
##########
src/backend/parser/cypher_clause.c:
##########
@@ -5293,6 +5293,75 @@ static A_Expr
*filter_vertices_on_label_id(cypher_parsestate *cpstate,
return makeSimpleA_Expr(AEXPR_OP, "=", (Node *)fc, (Node *)n, -1);
}
+/*
+ * Build a raw parser-node qual for relationship-type alternation patterns
+ * such as `[:A|B|C]`. Resolves each listed label against the graph catalog,
+ * keeps only those that exist as edge labels, and emits
+ *
+ * ag_catalog._extract_label_id(<edge>.id) IN (id_A, id_B, ...)
+ *
+ * which is semantically equivalent to `WHERE type(r) IN ['A','B',...]`.
+ * If no listed label resolves to an existing edge label, the qual
+ * short-circuits to FALSE.
+ *
+ * The pattern's `rel->label` is set to NULL by the parser action, so the
+ * edge variable is already resolved against the generic edge parent table;
+ * this qual narrows it back to the requested set.
+ */
+static Node *make_edge_label_alternation_qual(cypher_parsestate *cpstate,
+ transform_entity *entity)
+{
+ cypher_relationship *rel;
+ Node *id_field;
+ FuncCall *extract_id_fc;
+ List *id_consts = NIL;
+ ListCell *lc;
+
+ Assert(entity != NULL && entity->type == ENT_EDGE);
+ rel = entity->entity.rel;
+ Assert(list_length(rel->labels) > 1);
+
+ foreach (lc, rel->labels)
+ {
+ char *label_name = strVal((String *) lfirst(lc));
+ label_cache_data *lcd;
+ A_Const *c;
+
+ lcd = search_label_name_graph_cache(label_name, cpstate->graph_oid);
+ if (lcd == NULL || lcd->kind != LABEL_KIND_EDGE)
+ {
+ /* Unknown or non-edge label contributes no rows; keep walking
+ * the alternation so any valid sibling can still match. */
+ continue;
+ }
+
+ c = makeNode(A_Const);
+ c->val.ival.type = T_Integer;
+ c->val.ival.ival = lcd->id;
+ c->location = -1;
+ id_consts = lappend(id_consts, c);
+ }
+
+ /*
+ * No listed label corresponds to an existing edge label. Emit a constant
+ * FALSE qual so the edge produces no rows but planning still succeeds.
+ */
+ if (id_consts == NIL)
+ {
+ return make_bool_a_const(false);
+ }
Review Comment:
When all relationship types in an alternation are unknown, this returns
`make_bool_a_const(false)`, which builds an *agtype* literal and relies on
implicit agtype→boolean coercion in WHERE quals. It’s clearer and avoids
unnecessary casts to return a native boolean FALSE here since this node is
appended directly to the MATCH quals list.
##########
src/backend/parser/cypher_clause.c:
##########
@@ -5687,6 +5779,20 @@ static transform_entity
*transform_VLE_edge_entity(cypher_parsestate *cpstate,
transform_entity *vle_entity = NULL;
ParseNamespaceItem *pnsi;
+ /*
+ * Relationship-type alternation (`[:A|B*..]`) is not yet wired into the
+ * VLE path: the per-hop matcher built here ignores the new `labels`
+ * list, so silently accepting it would return all edges. Reject early
+ * until VLE matching learns to honour the alternation.
+ */
+ if (list_length(rel->labels) > 1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("variable length relationships with type alternation
is not yet supported"),
+ parser_errposition(&cpstate->pstate, rel->location)));
Review Comment:
Grammar in the error message: plural subject “relationships” should take
“are”, not “is”.
--
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]
