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
