Skip to content

support openCypher-like relationsiop-type edge alternation#2468

Open
eeshwarg wants to merge 2 commits into
apache:masterfrom
eeshwarg:relationship-type-alternation
Open

support openCypher-like relationsiop-type edge alternation#2468
eeshwarg wants to merge 2 commits into
apache:masterfrom
eeshwarg:relationship-type-alternation

Conversation

@eeshwarg

@eeshwarg eeshwarg commented Jul 7, 2026

Copy link
Copy Markdown

Apache AGE's MATCH grammar rejects openCypher's relationship-type alternation syntax - MATCH ()-[:A|B]->() fails at parse time with syntax error at or near "|". For example, a query like SELECT * FROM cypher('kg_s7', $$ MATCH (p)-[:WORKS_AT|REPORTS_TO]->(c) RETURN c $$) AS (c agtype); throws a syntax error. A workaround for this today is to run something like SELECT * FROM cypher('kg_s7', $$ MATCH (p)-[r]->(c) WHERE type(r) IN ['WORKS_AT','REPORTS_TO'] RETURN c $$) AS (c agtype);.

This change enables support for relationship-type alternation in accordance with the openCypher standard. Changes are made in the grammar, AST and during transformation from the AST to Postgres Query struct by adding the types to the quals list.

Variable-length relationship patterns with alternation ([:A|B*1..2]) are explicitly rejected with ERRCODE_FEATURE_NOT_SUPPORTED.

Testing:

  • Added regress test with cases with directed/undirected edges, and variable length edges

Future work:

  • openCypher also supports node-type alternation (like n:Person|Customer), which could be added to AGE in a future PR

Apache AGE's MATCH grammar rejects openCypher's relationship-type alternation syntax -
`MATCH ()-[:A|B]->()` fails at parse time with `syntax error at or near "|"`. For example,
a query like `SELECT * FROM cypher('kg_s7', $$ MATCH (p)-[:WORKS_AT|REPORTS_TO]->(c) RETURN c $$) AS (c agtype);`
throws a syntax error. A workaround for this today is to run something like
`SELECT * FROM cypher('kg_s7', $$ MATCH (p)-[r]->(c) WHERE type(r) IN ['WORKS_AT','REPORTS_TO'] RETURN c $$) AS (c agtype);`.

This change enables support for relationship-type alternation in accordance with the openCypher
standard. Changes are made in the grammar, AST and during transformation from the AST to Postgres
`Query` struct by adding the types to the `quals` list.

Variable-length relationship patterns with alternation `([:A|B*1..2])` are explicitly rejected
with `ERRCODE_FEATURE_NOT_SUPPORTED`.

Testing:
- Added regress test with cases with directed/undirected edges, and variable length edges

Future work:
- openCypher also support node-type alternation (like n:Person|Customer), which could be
added to AGE in a future PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds openCypher-compatible relationship-type alternation in MATCH patterns (e.g. ()-[:A|B]->()), by extending the Cypher grammar/AST and injecting an additional edge-label filter qual during transformation to a PostgreSQL Query. It also explicitly rejects variable-length edges combined with alternation.

Changes:

  • Extend the grammar and cypher_relationship AST node to represent multi-label edge patterns like [:A|B|C].
  • During MATCH transformation, inject a synthetic qual restricting matched edges to the alternation’s label-id set.
  • Add regression coverage for directed/undirected alternation, unknown labels, and the VLE rejection case.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/include/nodes/cypher_nodes.h Adds labels list to cypher_relationship to represent relationship-type alternation.
src/backend/parser/cypher_gram.y Parses `[:A
src/backend/parser/cypher_clause.c Injects an IN qual over _extract_label_id(edge.id) for alternation; rejects alternation with VLE.
src/backend/nodes/cypher_outfuncs.c Serializes the new labels field for cypher_relationship.
regress/sql/cypher_match_rel_label_alternation.sql Adds regression test cases covering alternation behavior and the VLE rejection.
regress/expected/cypher_match_rel_label_alternation.out Expected output for the new regression test.
Makefile Registers the new regression test in REGRESS.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +5349 to +5352
if (id_consts == NIL)
{
return make_bool_a_const(false);
}
Comment thread src/backend/parser/cypher_clause.c
@gregfelice

Copy link
Copy Markdown
Contributor

Hitting it tomorrow

@jrgemignani

Copy link
Copy Markdown
Contributor

@eeshwarg Please look at Copilot's suggestions. Additionally, how could this impact performance?

@eeshwarg

eeshwarg commented Jul 13, 2026

Copy link
Copy Markdown
Author

@eeshwarg Please look at Copilot's suggestions. Additionally, how could this impact performance?

In short, this shouldn't affect performance of already supported queries, but only adds support for a new query shape.

All the code changes are during plan generation, not execution time. Existing query shapes take an unchanged code path - the multi-label logic is gated behind list_length(labels) > 1, which is false for every single-label/unlabeled pattern, so they produce an identical Query tree and therefore an identical plan. The new path adds a single integer IN filter on a scan that already existed, with all label resolution done at plan time.

@gregfelice gregfelice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested this end-to-end against PostgreSQL 18.4. The implementation is correct — approving with comments. Three things below, one of which answers @jrgemignani's performance question with numbers.

What I verified

Built the branch and ran the full 42-test regress suite: all 42 pass, including the new cypher_match_rel_label_alternation. I ran unmodified master through the identical harness as a control to be sure nothing I saw was environmental.

Grammar is conflict-clean. Worth stating explicitly since the Makefile passes -Wno-error=conflicts-sr/-rr to bison and a new conflict would not fail the build on that flag alone. The branch measures exactly 7 shift/reduce + 3 reduce/reduce — identical to the %expect 7 / %expect-rr 3 declared on master for the GLR parser, and both directives are untouched. Since bison treats %expect as an exact count, the build would have hard-errored had this added even one conflict. So the new rel_labels rules introduce none.

Paths not covered by the new tests, which I probed by hand and which all behave correctly:

  • EXISTS((a)-[:A|B]->(b)) and a pattern expression in RETURN — both filter correctly (the qual does reach the pattern-expression transform).
  • OPTIONAL MATCH (a)-[:A|B]->(b) — filters correctly.
  • The VLE guard catches all four variable-length forms: *, *2, *1..2, *0... Nothing slips through.
  • Duplicate-edge qual, edge properties ([:A|B {k:1}]), edge variables and type(r) all work under alternation.
  • [:A|A] and unknown/vertex labels inside the alternation degrade sanely.
  • Unknown label ignored, all-unknown → zero rows, is the right openCypher semantics.

Also confirming the copy/read funcs are correctly left alone: cypher_relationship is registered with DEFINE_NODE_METHODS, whose copy_ag_node/read_ag_node throw, so it is never copied or read — touching only cypher_outfuncs.c is right, not an omission.

On Copilot's make_bool_a_const(false) note — that one is stylistic, not a bug. The agtype → boolean cast is registered AS IMPLICIT, so the all-labels-unknown FALSE qual coerces correctly in the WHERE list. No need to churn on it.

1. Performance — answering @jrgemignani's question

@eeshwarg, your gating argument is right and I confirmed it: existing query shapes are untouched, because list_length(labels) > 1 is false for every single-label and unlabeled pattern, and I verified the single-label plan still gets its index scan. No regression to existing queries.

But "adds a single integer IN filter on a scan that already existed" understates what happens. Setting label = NULL widens the edge RTE from one child table to the whole _ag_label_edge inheritance tree. That scan did not previously exist for this shape. On a graph with 11 edge labels:

Pattern Relations scanned Access method
[:A] 1 Index Scan (A_end_id_idx)
[:A|B] 12 (every edge table) Seq Scan + _extract_label_id(id) = ANY('{3,4}') post-filter

So cost is O(all edges in the graph), not O(|A| + |B|), and it degrades as the graph accumulates edge types. The _extract_label_id(...) IN (...) filter is applied after the scan and cannot prune the inheritance children.

This is not a blocker: it is exactly equivalent to the WHERE type(r) IN ['A','B'] workaround it replaces, so nobody is worse off. But as it stands the feature is nicer syntax rather than faster execution, and the PR description should say so instead of implying no performance impact.

The optimization that would make alternation genuinely worth having over the workaround is to lower it into an Append over only the named label RTEs, so each named label keeps its own indexes. Reasonable as a follow-up, alongside the node-label alternation you mention.

2. CREATE / MERGE error messages misdescribe the problem

The shared path_relationship_body rule means CREATE ()-[:A|B]->() and MERGE ()-[:A|B]->() now parse. Both correctly error rather than silently writing a wrongly-labeled edge — good — but because alternation sets label = NULL, they fall into the generic no-label branches:

CREATE (a:P)-[:A|B]->(b:P)   ->  ERROR: relationships must be specify a label in CREATE.
MERGE  (a:P)-[:A|B]->(b:P)   ->  ERROR: edges declared in a MERGE clause must have a label

The user did specify labels — they specified two. Please add explicit guards in transform_create_cypher_edge / transform_merge_cypher_edge mirroring the VLE one, e.g. "relationship type alternation is not supported in CREATE/MERGE", and add regress cases for both. There is no coverage for CREATE/MERGE + alternation today.

3. The comment on the new labels field is factually wrong

In cypher_nodes.h:

NOTE: This field MUST stay at the end of the struct. Several call sites type-pun an edge through cypher_node (a union member) and read shared prefix fields...

cypher_node and cypher_relationship are separate structs, not union members, and appending a field at the end of a struct cannot change the offsets of any preceding field regardless. The constraint described does not exist. Please drop or correct this before it misleads someone into designing around a rule that isn't real.


Nice piece of work overall — the transform is clean, it reuses the existing _extract_label_id precedent from filter_vertices_on_label_id, and the VLE guard is thorough. Items 2 and 3 are quick fixes; item 1 I'd just want reflected honestly in the description.

@MuhammadTahaNaveed

Copy link
Copy Markdown
Member

@eeshwarg This does not seem like the best approach going forward, especially for larger graphs with many edge labels. With labels A, B, C, D, E, MATCH ()-[r:A|B]->() should only scan A and B.

This PR appears to scan _ag_label_edge, which expands to A+B+C+D+E, and then filters by _extract_label_id(r.id) IN (...). That makes unrelated labels C/D/E part of the access path. The implementation should ideally build an Append/Union over only the resolved A and B relations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants