Thanks for the review :) > Instead of creating another view, I would try to add the WHERE clause in customer_us definition itself.
Great idea! Done in attached v2. The whole-pattern WHERE clause is now part of the existing customers_us definition, so there is no extra view. The predicate (p.price > 0) is deliberately trivial; as you say, the view is not queried anywhere, so only its deparsed definition matters. I also updated the comment above the view to mention that it covers WHERE clauses both on pattern elements and on the whole pattern. > Further, we usually don't drop any objects created by this test so that they can be tested in the 002_pg_upgrade test. Please observe that customer_us view is not dropped hmm makes sense, will take care of it v2 patch is attached. Regards, Dhruv On Thu, 30 Jul 2026 at 08:32, Ashutosh Bapat <[email protected]> wrote: > > > On Sat, Jul 25, 2026 at 2:57 PM Chauhan Dhruv <[email protected]> > wrote: > >> Hi, >> >> *Issue:* >> The `get_graph_pattern_def()` function omits a leading space before the >> WHERE keyword when a GRAPH_TABLE graph pattern includes a WHERE clause. >> This produces incorrectly formatted SQL during reverse-parsing. e.g. >> >> ... (o IS orders)WHERE (c.address = 'US'::text) COLUMNS ... >> >> Notice the missing space between ) and WHERE. >> >> *Root Cause:* >> The pattern-level WHERE deparse doesn't add a leading space, while the >> element-level WHERE deparse already does. This inconsistency creates >> malformed output. >> >> *Solution:* >> The patch changes the pattern-level branch to emit " WHERE " (with >> leading space) instead of "WHERE ", matching the element-level behavior. >> Although the SQL still re-parses correctly (making this cosmetic), the >> formatted output is now correct. >> >> > I wouldn't call it "incorrectly formatted SQL" since it gets parsed > without any error and has the same semantics as with space. I agree that > the deparsed string is not readable. The fix looks good to me. > > >> Also adds a regression test for whole-pattern WHERE clauses. make check is >> green. Patch is against master. >> >> > Instead of creating another view, I would try to add the WHERE clause in > customer_us definition itself. I don't think we can add a WHERE clause > which has any meaningful effect on the contents on the view. It will still > show the effect of deparsing and since the view is not used in any query, > its actual contents can be ignored. > > Further, we usually don't drop any objects created by this test so that > they can be tested in the 002_pg_upgrade test. Please observe that > customer_us view is not dropped. > > -- > Best Wishes, > Ashutosh Bapat >
From bc00b2979ec0ece583ee9c049820d24d5837871e Mon Sep 17 00:00:00 2001 From: Dhruv Chauhan <[email protected]> Date: Sat, 25 Jul 2026 14:28:48 +0530 Subject: [PATCH v2] Fix missing space before WHERE in GRAPH_TABLE deparse get_graph_pattern_def() emitted the pattern-level WHERE keyword as "WHERE " with no leading space, so reverse-parsing produced output like "(o IS orders)WHERE (...)". The element-level WHERE deparse in get_path_pattern_expr_def() already prepends a separating space; the pattern-level branch was inconsistent with it. The output still re-parses to the same tree, so this is cosmetic, but the deparsed text is wrong. Emit " WHERE " to match. For test coverage, add a whole-pattern WHERE clause to the existing customers_us view, which is already reverse-parsed with pg_get_viewdef(). --- src/backend/utils/adt/ruleutils.c | 2 +- src/test/regress/expected/graph_table.out | 21 +++++++++++---------- src/test/regress/sql/graph_table.sql | 5 +++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b44b7a78d..e697fb66da 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8134,7 +8134,7 @@ get_graph_pattern_def(GraphPattern *graph_pattern, deparse_context *context) if (graph_pattern->whereClause) { - appendStringInfoString(buf, "WHERE "); + appendStringInfoString(buf, " WHERE "); get_rule_expr(graph_pattern->whereClause, context, false); } } diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out index 46566b2e32..b7a6182457 100644 --- a/src/test/regress/expected/graph_table.out +++ b/src/test/regress/expected/graph_table.out @@ -965,13 +965,14 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e -- GRAPH_TABLE in views -- The query in the view definition is intentionally complex to test one view with many --- features like label disjunction, lateral references, WHERE clauses in graph --- patterns. +-- features like label disjunction, lateral references, WHERE clauses on graph +-- pattern elements as well as on the whole graph pattern. CREATE VIEW customers_us AS SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) + WHERE p.price > 0 COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; -- Dropping properties or labels used by a view is not allowed @@ -993,14 +994,14 @@ DETAIL: view customers_us depends on property price of property graph myshop HINT: Use DROP ... CASCADE to drop the dependent objects too. -- ruleutils reverse parsing SELECT pg_get_viewdef('customers_us'::regclass); - pg_get_viewdef ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- - SELECT g.customer_name, + - g.product_name, + - g.price, + - g.a + - FROM x1, + - GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) COLUMNS (c.name AS customer_name, p.name AS product_name, p.price AS price, x1.a AS a)) g+ + pg_get_viewdef +----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + SELECT g.customer_name, + + g.product_name, + + g.price, + + g.a + + FROM x1, + + GRAPH_TABLE (myshop MATCH (c IS customers WHERE (((c.address)::text = 'US'::text) AND (c.customer_id = x1.a)))-[IS customer_orders|customer_wishlists]->(l IS orders|wishlists)-[IS list_items]->(p IS products) WHERE (p.price > (0)::numeric) COLUMNS (c.name AS customer_name, p.name AS product_name, p.price AS price, x1.a AS a)) g+ ORDER BY g.customer_name, g.product_name; (1 row) diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql index 3fb0e50ddb..85298f9396 100644 --- a/src/test/regress/sql/graph_table.sql +++ b/src/test/regress/sql/graph_table.sql @@ -539,13 +539,14 @@ SELECT * FROM GRAPH_TABLE (g4 MATCH (s WHERE s.id = 3)-[e]-(d) COLUMNS (s.val, e -- GRAPH_TABLE in views -- The query in the view definition is intentionally complex to test one view with many --- features like label disjunction, lateral references, WHERE clauses in graph --- patterns. +-- features like label disjunction, lateral references, WHERE clauses on graph +-- pattern elements as well as on the whole graph pattern. CREATE VIEW customers_us AS SELECT g.* FROM x1, GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US' AND c.customer_id = x1.a) -[IS customer_orders | customer_wishlists ]-> (l IS orders | wishlists)-[ IS list_items]->(p IS products) + WHERE p.price > 0 COLUMNS (c.name AS customer_name, p.name AS product_name, p.price, x1.a AS a)) g ORDER BY customer_name, product_name; -- Dropping properties or labels used by a view is not allowed -- 2.34.1
