"Heikki Linnakangas" <heikki@enterprisedb.com> 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.)
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.
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?
regards, tom lane