From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | kathy(dot)forte(at)2ndquadrant(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: autogenerated column names + views are a dump hazard |
Date: | 2015-03-03 15:58:05 |
Message-ID: | 20150303155805.GB12445@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2015-03-03 09:39:03 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2015-03-03 15:58:28 | Re: New CF app deployment |
Previous Message | Andres Freund | 2015-03-03 15:49:22 | Comparing primary/HS standby in tests |