Re: page macros cleanup (ver 04)

Tom Lane napsal(a):
> "Heikki Linnakangas" <[email protected]> writes:
>> ...  That macro is actually doing the 
>> same thing as PageGetContents, so I switched to using that. As that 
>> moves the data sligthly on those bitmap pages, I guess we'll need a 
>> catversion bump.
> 
> I'm amazed that Zdenek didn't scream bloody murder about that.  You're
> creating a work item for in-place-upgrade that would not otherwise
> exist, in exchange for a completely trivial bit of code beautification.
> (The same can be said of his proposed change to hash meta pages.)

:-) Yeah, These changes break in-place-upgrade on hash indexes and invokes 
reindexing request. I have had several reasons why I didn't complaint about it:

1) IIRC, hash function for double has been change
2) there is ongoing project to improve hash index performance -> completely 
redesigned content
3) hash index is not much used (by my opinion) and it affect only small group of 
users

> I'm planning to go over this patch today and apply it sans the parts
> that would require catversion bump.  We can argue later about whether
> those are really worth doing, but I'm leaning to "not" --- unless Zdenek
> says that he has no intention of making in-place-upgrade handle hash
> indexes any time soon.

Thanks for applying patch. I think that hash index "upgradebility" is currently 
broken or it will be with new hash index improvement. But if I think about it it 
does not make sense to break compatibility by this patch first. I will prepare 
patch which will be upgrade friendly. And if we will reimplement hash index 
soon, than we can clean a code.

> BTW, after further thought about the PageGetContents() situation:
> right now we can change it to guarantee maxalignment "for free",
> since SizeOfPageHeaderData happens to be maxaligned on all platforms
> (this wasn't the case as recently as 8.2).  So I'm thinking we should
> do that.  There's at least one place that thinks that PageGetContents
> is the same as page + SizeOfPageHeaderData, but that's easily fixed.
> On balance it seems like hidden assumptions about alignment are a bigger
> risk than assumptions about that offset --- anyone want to argue the
> contrary?

I think it is OK and I seen that you already applied a patch.

		Thanks Zdenek



-- 
Zdenek Kotala              Sun Microsystems
Prague, Czech Republic     http://sun.com/postgresql


-- 
Sent via pgsql-patches mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches