Re: BUG #18589: pg_get_viewdef returns wrong query

I wrote:
> Yeah, I was uncertain whether to bother with that detail. It's true
> that there's no bug in GROUP BY because the parser prefers SQL99
> semantics in that case, but maybe we shouldn't have this code assuming
> that? And it wasn't easy to do with the special_exprkind approach.
> But it is pretty easy with the two-bools approach, so I fixed that in
> the attached v2.

I was just about to commit v2 when I realized that it's wrong, because
what we need to be checking against is the output column name(s) that
get_target_list() will print, and that's not necessarily tle->resname.
You can in fact make v2 misbehave with ALTER VIEW RENAME COLUMN, as
shown by the added test cases in attached v3.

That complicates matters greatly because it means that we also need
access to the current get_query_def call's resultDesc. To avoid
cluttering things impossibly, I decided that resultDesc ought to be
passed down in the deparse_context not as a separate parameter, and
while doing that it seemed better to do the same with colNamesVisible.
(There is one place that has to save-n-restore colNamesVisible to
make that work, but overall it's less code and clutter.)

It's slightly annoying that there are now three places that know
how get_target_list chooses output column names. I thought about
refactoring to get that down to one place, but couldn't really
find anything that didn't net out as more code and ugliness.

Also, while looking at this I realized that get_select_query_def's
habit of saving-and-restoring windowClause and windowTList is
completely pointless: it has only one caller and that one has
just set up a fresh deparse_context, which won't be used anymore
afterwards. If there were a code path in which that did something,
the lack of save-and-restore logic in get_update_query_def and
friends would likely be a bug. So I just dropped it. I believe
that it's not really necessary to save-n-restore inGroupBy or
varInOrderBy either (rather than just reset them to false).
But those cases aren't quite as clear-cut, and resetting to false
isn't much cheaper than restoring a saved value, so I left those
bits as-is.

regards, tom lane