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 (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to