linhongyu510 commented on PR #2559:
URL: https://github.com/apache/age/pull/2559#issuecomment-6140430106
Thanks for the careful review — and for the exact reproduction. I verified
the default-label case against a local build.
## Root cause
In `is_redundant_edge_type_qual()`, a pattern edge on the default label
`_ag_label_edge` was treated like a static-label edge, so `type(r) =
'_ag_label_edge'` was judged redundant and elided. But `_ag_label_edge` is the
catch-all edge label that scans every child edge label, so the predicate is
**not** redundant: dropping it returns the actual child edge type instead of
filtering to zero rows.
## Fix
`a8cb30bc fix(parser): do not elide type() qual on default label edge` —
`is_redundant_edge_type_qual()` now returns `false` when the pattern edge is
the default label (`IS_DEFAULT_LABEL_EDGE(static_label)`), so the filter is
retained.
## Reproduction (your exact SQL)
```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);
```
Built AGE against local PostgreSQL 18.6 (`REL_18_STABLE`), bison 3.8.2:
| Build | Result |
|---|---|
| `4d5707d6` (before fix) | `edge_type = "P"` — **1 row** (the reported bug)
|
| `a8cb30bc` / head | `(0 rows)` — **empty** (correct, matches master) |
## Regression coverage
Added the default-label case to `regress/sql/expr.sql` in `3d3741b3` (with
refreshed `regress/expected/expr.out`). It fails before the fix (returns `"P"`)
and passes after (`0 rows`).
`make installcheck` on head: **42/43 tests pass**. The single local failure
is `age_load`, which errors with `You can only load files located in
[/tmp/age/]` — on macOS `/tmp` resolves to `/private/tmp`, so `realpath` no
longer matches the hardcoded `/tmp/age/` prefix. This is a platform/environment
issue unrelated to this parser change (it passes on the Linux CI runner), and I
did not modify that test.
Happy to adjust the test placement or wording. Could you re-review at head
`3d3741b3`?
---
*Note: this diagnosis, the local reproduction/build, and the regression-test
addition were performed with AI assistance; all results above are from measured
local `make installcheck` runs and the exact psql queries shown.*
--
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]
