Andrew Dunstan <andrew.duns...@2ndquadrant.com> writes:
> I left that for a separate exercise. I think this does things the way 
> you want. I must admit to being a bit surprised, I was expecting you to 
> have more to say about the upgrade function than the pg_dump changes :-)

Well, the upgrade function is certainly a bit ugly (I'm generally allergic
to patches that result in a large increase in the number of #includes in
a file --- it suggests that the code was put in an inappropriate place).
But I don't see a good way to make it better, at least not without heavy
refactoring of StoreAttrDefault so that some code could be shared.

I think this is probably OK now, except for one thing: you're likely
to have issues if a dropped column has an attmissingval, because often
the value won't agree with the bogus "integer" type we use for those;
worse, the array might refer to a type that no longer exists.
One way to improve that is to change

                              "CASE WHEN a.atthasmissing THEN a.attmissingval "
                              "ELSE null END AS attmissingval "

to

                              "CASE WHEN a.atthasmissing AND NOT a.attisdropped 
THEN a.attmissingval "
                              "ELSE null END AS attmissingval "

However, this might be another reason to reconsider the decision to use
anyarray: it could easily lead to situations where harmless-looking
queries like "select * from pg_attribute" fail.  Or maybe we should tweak
DROP COLUMN so that it forcibly resets atthasmissing/attmissingval along
with setting atthasdropped, so that the case can't arise.

Like Andres, I haven't actually tested, just eyeballed it.

                        regards, tom lane

Reply via email to