[DEPARSE] A COLLATE clause in the URI of XMLNAMESPACES does not read back
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.
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:t254302psql -h localhost -U postgresBuilt from patchset v1 (message #1), October 24, 2026 at 12:32 PM.
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 t254302_1 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 t254302_1 && git checkout t254302_1Patchset v1 (message #1) is on t254302_1
Hi hackers,
The namespace URI in XMLNAMESPACES is a b_expr in the grammar, so an
expression with a COLLATE clause has to be in parentheses there.
Pretty-printed output leaves them out, so the definition of a view
using one cannot be read back. The two attached patches add a test
and fix it.
The problem
-----------
create view v as
select * from xmltable(
xmlnamespaces(('http://x.y' collate "C") as p),
'/p:a' passing (null::xml) columns v text path 'p:b');
before: XMLNAMESPACES ('http://x.y'::text COLLATE "C" AS p)
ERROR: syntax error at or near "COLLATE"
after: XMLNAMESPACES (('http://x.y'::text COLLATE "C") AS p)
Only pretty-printed output is affected, such as pg_get_viewdef(...,
true) and psql's \d+. Outside pretty mode the COLLATE expression
prints its own parentheses, so pg_dump is fine.
The fix
-------
get_xmltable() printed the URI with get_rule_expr() as it was. 0002
puts it in parentheses, after XMLNAMESPACES( as well as after DEFAULT,
unless it is a plain Const. A constant is the usual case and needs
none, so its output does not change. Any other expression gets them
even where it would not need them, such as an operator expression;
that output read back before and still does. This is what ruleutils.c
already does for b_expr and c_expr arguments elsewhere, such as those
of POSITION and XMLEXISTS.
Outside pretty mode such a URI now gets two pairs of parentheses,
because the COLLATE expression prints its own there. That is how
other operands are printed in that mode, and it reads back.
Testing
-------
0001 adds a view to xml.sql, prints it with pg_get_viewdef(..., true)
and recreates the view from that output. 0001 has the output before
the fix, where the recreation fails, and 0002 updates it.
Back branches
-------------
XMLNAMESPACES has been printed this way since XMLTABLE came in 10, so
I think this should be fixed back to 14. The code change applies to
14-19 as is. The test applies to 17 and later; in 14-16 it needs
another place in xml.sql, and its expected output may differ.
This came out of the same AI-assisted audit of ruleutils.c as my
earlier [DEPARSE] reports.
Best regards,
Henson
Attachments:
t254302_1v1-0001-Add-a-test-for-a-COLLATE-clause-in-the-URI.patchapplication/octet-stream; name=v1-0001-Add-a-test-for-a-COLLATE-clause-in-the-URI.patchDownload+28-1
v1-0002-Parenthesize-the-URI-of-XMLNAMESPACES.patchapplication/octet-stream; name=v1-0002-Parenthesize-the-URI-of-XMLNAMESPACES.patchDownload+16-7
Hi Henson,
Thanks for the patch. I read the report and the patch, and tested
v1 hands-on (the hands-on testing below was done with AI assistance;
the code reading and this review are mine).
Code reading: the URI of XMLNAMESPACES is a b_expr, so it cannot
carry a COLLATE, and get_xmltable() was the one place in that function
printing an expression without parentheses -- the ROWS expression,
the PASSING document and the column expressions are all wrapped
already. In pretty mode a CollateClause drops its own parentheses,
so the printed definition is not valid input. Parenthesizing the URI
there, in both the AS-name and the DEFAULT branch, fixes it at the
right layer, and the plain-Const exception keeps the common case free
of output churn. Parenthesizing in non-pretty mode as well gives two
pairs of parentheses there, which reads back fine and is consistent
with how other operands in this function are treated.
Reproduction on current master: with views whose namespace URI carries
a COLLATE (named namespace, DEFAULT namespace, one element of a mixed
list, and a function-call URI), pg_get_viewdef(..., true) prints the
COLLATE bare, and recreating the views from those definitions fails
with 'syntax error at or near "COLLATE"' - 6 of the 12 objects in
my test battery. The non-pretty definitions round-trip, and a full
pg_dump/restore is clean on master, matching your report that this
is a pretty-printing problem only. I also confirmed that functions
are not affected, since pg_get_functiondef prints the body non-pretty.
With v1 applied, every object round-trips from both the pretty and
the non-pretty definition, with the definitions byte-identical before
and after recreation, and a pg_dump/restore is clean. Objects created
before the patch are fixed by the patched deparser alone, as expected
for a ruleutils fix. Plain-constant URIs print exactly as before
(a literal with a cast folds to a Const and is also unchanged);
operator-expression URIs gain parentheses, as your commit message
notes, and still read back. One extra form I tried: a LATERAL view
whose namespace URI is a column reference with a COLLATE also reads
back and executes correctly with the patch.
One small coverage note: the new test covers the pretty, named-AS
form. The DEFAULT form and the non-pretty output are exercised by my
testing above but are not in the patch's tests; a DEFAULT line in the
new test would be a nice addition, but I do not consider it a blocker.
I ran installcheck-world against the patched build: core regression
239/239, core isolation 134/134, contrib 206/206 + 20/20 isolation,
test modules 71/71 + 24/24 isolation - all pass.
This looks complete and correct to me.
~ Prashant
Hi Prashant,
Thank you for the review and the testing.
One small coverage note: the new test covers the pretty, named-AS
form. The DEFAULT form and the non-pretty output are exercised by my
testing above but are not in the patch's tests; a DEFAULT line in the
new test would be a nice addition, but I do not consider it a blocker.
Good point. I will extend the test and post a v2.
Best regards,
Henson