Re: [PATCH] regression test: view and rule definitions must round trip

On Monday, October 26th, 2026 at 2:36 PM, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:

> On Mon, Oct 26, 2026 at 3:13 PM Greg Burd <greg(at)burd(dot)me> wrote:
> >
> > Hello hackers,
> >
> > PostgreSQL can show you the definition of a view with pg_get_viewdef()
> > (and a rule with pg_get_ruledef()). This is how pg_dump writes views
> > into a backup. The rule that has to hold: the text it gives back must be
> > valid SQL that recreates the same view. If it hands you SQL that won't
> > parse, your backup won't restore. My test checks that this holds for a
> > batch of tricky view and rule definitions, so a future code change can't
> > quietly break it.
> >
> > But this is fragile because when you write a view, PostgreSQL doesn't
> > store your original SQL text, it stores a parsed tree, then reconstructs
> > the SQL text from that tree whenever you ask for the definition. That
> > reconstruction step (the "deparser") has to get the spelling exactly
> > right (every parenthesis, every quote, every keyword). It mostly does,
> > but there are corners where it drops something, and then the
> > reconstructed text is subtly invalid.
> >
> > I was working with a differential selectivity test framework and I ran
> > into the three real examples (all shipped as bugs at some point):
> >
> > - A COLLATE clause inside an XMLNAMESPACES URI came back without
> > parentheses, and the grammar doesn't allow a bare COLLATE there — so
> > the recreated view failed to parse with "syntax error at or near
> > COLLATE".
> >
> > - Operators on ROW() constructors could read back wrong.
> >
> > - A rule whose action uses a table alias named "set" (a reserved-ish
> > word) came back unquoted as UPDATE t set SET ..., which is a syntax
> > error.
> >
> > The common thread: the reconstruction is a separate piece of code from
> > the parser, so the two can drift, and it is very hard to notice until a
> > specific weird definition trips it.
> >
> > Using the framework I've been developing I was able to create a series
> > of teststhat try to codify this contract so as to be more noticable in
> > the future if/when some change overlooks this requirement. Essentially,
> > for each view in the corpus it does a round-trip and asserts three
> > things:
> >
> > 1. It re-parses: take the definition pg_get_viewdef() hands back, feed
> > it to CREATE VIEW again; it must succeed (not a syntax error).
> >
> > 2. Same rows: the recreated view must return the same data as the
> > original (the text didn't just parse, it means the same thing).
> >
> > 3. Fixed point: deparse the recreated view and compare to the deparse
> > of the original; they must be byte-identical (reconstructing it a second
> > time doesn't drift further).
> >
> > Each case prints just ok or FAIL <why>, so the expected-output file
> > stays small and doesn't care how the deparser chooses to spell a given
> > view — only that the round-trip holds.
> >
> > There are round-trip-ish checks scattered through rules.sql,
> > create_view.sql, xml.sql but they work by someone adding a tricky view,
> > eyeballing that \d+ looks right, and moving on. AFAICT, there's no
> > single test that says "take this whole corpus of known-nasty shapes and
> > assert the round-trip property for every one." That gap is exactly why
> > the XMLNAMESPACES bug and its three siblings shipped: no systematic
> > guard caught them. The test intends to fills that gap with one generic
> > oracle instead of dozens of ad-hoc eyeball checks.
>
> If those views are left behind by the test in the regression database
> 002_pg_upgrade will dump those views, restore them, dump them again
> and compare the original dump and restored dump. That should cover the
> round trip you are trying to test.

I'm not sure what you're suggesting, yes that is true but does that mean
that you'd prefer that method for validation over what I've proposed?

> Are the views testing the features above left behind by the testcase
> or dropped?

They are in a schema and removed at the end of the test.

> --
> Best Wishes,
> Ashutosh Bapat

The purpose of the suggested patch is to simply how we codify and ensure
that this kind of bug isn't re-introduced down the road.

best.

-greg