Re: ALTER TABLE .. ALTER COLUMN .. ERROR: attribute .. has wrong type

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER TABLE .. ALTER COLUMN .. ERROR: attribute .. has wrong type
Date: 2017-01-06 21:33:46
Message-ID: 20170106213346.vro3rrghuzabnxmv@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > Alvaro Herrera wrote:
> >> Here's a first attempt at fixing this. It makes the test pass, but I
> >> have the feeling that more complex ones might need more work.
>
> > Here's another one with three main differences:
>
> Hmm. The bespoke code for constructing the attno map bothers me;
> surely there is existing code that does that? If not, it'd still
> make more sense to factor it out, I think, because there will be
> other needs for it in future.

There isn't any that I could find -- all the existing callers of
map_variable_attnos build their map in other ways (while walking an
attribute array at construction time).

So I did as you suggest, 'cause it sounds like a good idea, but the
problem crops up of where to put the new function. The obvious
candidate is rewriteManip.c next to map_variable_attnos itself, but the
include creep is a bit bothersome -- maybe it indicates that the new
function should be elsewhere. But then, the whole of rewriteManip seems
not terribly well delimited to the rewriter itself but just an assorted
collection of walkers, mutators, and similar utilities used by code all
over the place, so perhaps this is not a problem.

I also modified the algorithm to use the relcache instead of walking the
child's attribute list for each parent attribute (that was silly).

Here's the new version.

> Otherwise, this seems sound in terms of fixing the observed problem,
> but what are the implications for event triggers exactly? Does a
> trigger see only the original expression, or only the modified expression,
> or ???

My rationale when writing the event trigger code was that each command
would only be published once, for the parent table, not recursively for
each child. So only the original expression should be seen. I have not
yet verified the actual behavior in the differing attnums case. One
problem at a time ...

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
fix-altertype-3.patch text/plain 8.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2017-01-06 22:04:00 Re: Cluster wide option to control symbol case folding
Previous Message Robert Haas 2017-01-06 21:03:55 _hash_addovflpage has a bug