Fix missing space before WHERE in `GRAPH_TABLE` deparse
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253190psql -h localhost -U postgresBuilt from patchset v3 (message #3), August 02, 2026 at 11:38 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253190_3 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253190_3 && git checkout t253190_3Patchset v3 (message #3) is on t253190_3
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
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
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
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 droppedhmm makes sense, will take care of it
v2 patch is attached.
Committed, thanks.