On Thu, Dec 10, 2009 at 1:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I'm not an expert on this area of the code, but can we just ignore
>> isNatural and usingClause when deparsing?
>
> No. =A0These properties are *not* ignorable because doing so changes the
> set of returned columns --- you should get only one column out not two.
>
> My reading of the spec is that USING (and therefore NATURAL) is defined
> to join identically named columns. =A0Therefore, renaming one of the input
> columns as the OP did *should* indeed *must* break the view. =A0The probl=
em
> is not how to make it work, it's how to give an error message that
> doesn't look like an internal failure.
That seems ugly and unnecessary. I think we might be able to define
ourselves out of this problem. We don't guarantee (and have never
guaranteed) that selecting from a stored view will produce the same
results as re-executing the original query. For example, * refers the
list of columns at definition-time, not execution-time, and if a
column is renamed, the view still refers to the same column; it
doesn't start crashing, nor would we want it to. Similarly, here, the
USING is internally converted to an equality join on the two columns,
and the ambiguous output column is, I think, resolved in favor of one
of them. I think we can just say that that conversion happens in toto
at parse-time, just as the *-to-column-list conversion and the
column-name-to-column-reference conversions do. This seems like a
significantly more useful behavior and as a fringe benefit it
simplifies the code.
Moreover, it's basically what we're already doing for so long as the
view remains safely stored in the database; it just becomes
undumpable. If we adopt your solution, backpatching it might break
working applications and not backpatching it will leave residual
breakage when people try to upgrade. If we just make the dumping code
work the same way the executor already does, we don't have that
problem.
...Robert
