On 06/21/2018 04:41 PM, Tom Lane wrote:
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.
I moved the bulk of the code into a function in heap.c where it seemed
right at home.
I added removal of missing values from dropped columns, as well as
qualifying the test as above.
The indent issue raised by Alvaro is also fixed. I included your patch
fixing the regression tests.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
