Re: autogenerated column names + views are a dump hazard

On 2015-03-03 09:39:03 -0500, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > For this case it seems easiest if we'd make get_rule_expr() (and some of
> > its helpers) return whether an implicit cast has been added.
>
> Aside from being pretty ugly, that doesn't seem particularly
> bulletproof.

Right.

> It might deal all right with this specific manifestation, but now that
> we've seen the problem, who's to say that there aren't other ways to get
> bit? Or that some innocent future change to FigureColname() might not
> create a new one?  It's not like the behavior of that function was handed
> down on stone tablets --- we do change it from time to time.

Sure it'd not be a guarantee, but what is?

> I wondered whether there was a way to directly test whether
> FigureColname() gives a different result from what we have recorded
> as the effective column name.  Unfortunately, it wants a raw parse tree
> whereas what we have is an analyzed parse tree, so there's no easy way
> to apply it and see.

Additionally the casts added by get_rule_expr() show up in neither tree,
so that'd not in itself help.

> Now, we do have the deparsed text of the column expression at hand,
> so in principle you could run that back through the grammar to get a raw
> parse tree, and then apply FigureColname() to that.  Not sure what that
> would be like from a performance standpoint, but it would provide a pretty
> robust fix for this class of problem.

Yea, I've wondered about that as well. Given that this is pretty much
only run during deparsing I don't think the overhead would be relevant.

> And on the third hand ... doing that would really only be robust as long
> as you assume that the output will be read by a server using exactly the
> same FigureColname() logic as what we are using.  So maybe the whole idea
> is a bad one, and we should just bite the bullet and print AS clauses
> always.

I think this is the way to go though. There's different extremes we can
go to though - the easiest is to simply remove the attname = "?column?"
assignment from get_target_list(). That way plain Var references (aside
from whole row vars/subplans and such that get_variable() deals with)
don't get a forced alias, but everything else does.  That seems like a
good compromise between readability and safety. get_rule_expr() deals
with most of the "nasty" stuff.

I did this, and the noise in the regression test output isn't too
bad. Primarily that a fair number of of EXISTS(SELECT 1 .. ) grow an
alias.

> Or we could consider printing them always if the "pretty" flag isn't
> on.  IIRC, pg_dump doesn't enable that.

Not a fan of that, I'd rather not output queries that can't be executed.

Greetings,

Andres Freund

--Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services