Re: In-place upgrade: catalog side - Mailing list pgsql-hackers

Greg Smith <gsmith@gregsmith.com> writes:

> On Thu, 4 Dec 2008, Gregory Stark wrote:
>
>> They all seem functional ideas. But it seems to me they're all ideas that
>> would be appropriate if this was a pgfoundry add-on for existing releases.
>
> I was mainly trying to target things that would be achievable within the
> context of the existing shell script.  I think that we need such a script that
> does 100% of the job and can be tested ASAP.  If it's possible to slice the
> worst of the warts off later, great, but don't drop focus from getting a
> potential candidate release done first.

I suggested it because I thought it would be easier and less messy though.

>> How about adding a special syntax for CREATE TABLE which indicates to include
>> a dropped column in that position? Then pg_dump could have a -X 
>> option to include those columns as placeholders...This is an internal syntax
>> so I don't see any reason to bother making new keywords just to pretty up the
>> syntax. I don't see a problem with just doing something like "NULL COLUMN
>> (2,1)"
>
> It's a little bit ugly, but given the important of in-place upgrade I would
> think this is a reasonable hack to consider; two questions:

Well it's ugly but it seems to me that it would be a whole lot more ugly to
have a whole pile of code which tries to adjust the table definitions to
insert "dropped" columns after the fact.

> -Is there anyone whose clean code sensibilities are really opposed to adding
> such a syntax into the 8.4 codebase?

Incidentally I got this wrong in my previous email. If we're aiming at
8.4->8.5 as the first in-place update then we actually don't need this in 8.4
at all. The recommended path is always to use the *new* pg_dump to dump the
old database even for regular updates and there would be no problem making
that mandatory for an in-place update. So as long as the 8.5 pg_dump knows how
to dump this syntax and the 8.5 create parser knows what to do with it then
that would be sufficient.

> -If nobody has a beef about it, is this something you could draft a patch for?
> I'm going to be busy with the upgrade script stuff and don't know much about
> extending in this area anyway.

It doesn't sound hard off the top of my head. CREATE TABLE is a bit tricky
though, I'll check to make sure it works.

>> Actually removing the attribute is downright hard. You would have to have the
>> table locked, and squeeze the null bitmap -- and if you crash in the middle
>> your data will be toast.
>
> Not being familiar with the code, my assumption was that it would be possible
> to push all the tuples involved off to another page as if they'd been updated,
> with WAL logging and everything, similarly to the ideas that keep getting
> kicked around for creating extra space for header expansion. Almost the same
> code really, just with the target of moving everything that references the dead
> column rather than moving just enough to create the space needed.  Actually
> doing the upgrade on the page itself does seem quite perilous.

I'm sorry, I think I misunderstood the original idea, what you're talking
about makes a lot more sense now. You want to save the space of the dead
column by replacing it with NULL, not remove it from the table definition.

That should be possible to do in vacuum or some other operation that has the
vacuum lock without locking the table or introducing new tuples. Whatever does
it does need the tuple descriptor which Vacuum normally doesn't need though.
The page conversion time might be a good time since it'll need to deform all
the tuples and re-form them anyways.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication
support!