From: Zdenek Kotala <Zdenek.Kotala@Sun.COM>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <heikki@enterprisedb.com>
Cc: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: PostgreSQL-patches <pgsql-patches@postgresql.org>
Subject: Re: page macros cleanup (ver 04)
Date: Mon, 21 Jul 2008 21:19:25 +0200
Message-ID: <4884E13D.3070109@sun.com> (raw)
In-Reply-To: <13370.1215975739@sss.pgh.pa.us>
References: <48529B83.9010907@sun.com>
<2e78013d0807030450m4cab60d0jd99d1acce59fae56@mail.gmail.com>
<486DD1D1.50604@sun.com>
<2e78013d0807040150w393ad72codaf06a6ac12a83ce@mail.gmail.com>
<486DF644.4070604@enterprisedb.com>
<2e78013d0807040350m3b2913cfkceaa70d8f9d697c2@mail.gmail.com>
<486E01B0.9050100@enterprisedb.com>
<2e78013d0807040404q5ff7f7faq9f87a39e0f0552fa@mail.gmail.com>
<486E138F.4060900@sun.com>
<4874A6E0.9070909@enterprisedb.com>
<13370.1215975739@sss.pgh.pa.us>
Tom Lane napsal(a):
> "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.)
:-) 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: Zdenek.Kotala@Sun.COM, tgl@sss.pgh.pa.us, heikki@enterprisedb.com, pavan.deolasee@gmail.com
Subject: Re: page macros cleanup (ver 04)
In-Reply-To: <4884E13D.3070109@sun.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox