Fix missing space before WHERE in `GRAPH_TABLE` deparse

Started by Chauhan Dhruv9 days ago4 messageshackers
Jump to latest
#1Chauhan Dhruv
chauhandhruv351@gmail.com

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.

Also adds a regression test for whole-pattern WHERE clauses. make check is
green. Patch is against master.

Thanks,
Dhruv Chauhan

Attachments:

v1-0001-Fix-missing-space-before-WHERE-in-GRAPH_TABLE-dep.patchapplication/x-patch; name=v1-0001-Fix-missing-space-before-WHERE-in-GRAPH_TABLE-dep.patchDownload+22-2
#2Ashutosh Bapat
ashutosh.bapat.oss@gmail.com
In reply to: Chauhan Dhruv (#1)
Re: Fix missing space before WHERE in `GRAPH_TABLE` deparse

On Sat, Jul 25, 2026 at 2:57 PM Chauhan Dhruv <chauhandhruv351@gmail.com>
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

#3Chauhan Dhruv
chauhandhruv351@gmail.com
In reply to: Ashutosh Bapat (#2)
Re: Fix missing space before WHERE in `GRAPH_TABLE` deparse

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 <ashutosh.bapat.oss@gmail.com>
wrote:

Show quoted text

On Sat, Jul 25, 2026 at 2:57 PM Chauhan Dhruv <chauhandhruv351@gmail.com>
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

Attachments:

v2-0001-Fix-missing-space-before-WHERE-in-GRAPH_TABLE-dep.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Fix-missing-space-before-WHERE-in-GRAPH_TABLE-dep.patchDownload+15-9
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Chauhan Dhruv (#3)
Re: Fix missing space before WHERE in `GRAPH_TABLE` deparse

On 30.07.26 13:20, Chauhan Dhruv wrote:

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.

Committed, thanks.