Re: unnecessary code in_bt_split

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:
> Tom Lane napsal(a):
>> Not violating a perfectly good abstraction?

>   By my opinion It would be better to have three functions:

> PageCreateTempPage - only allocate memory and call pageinit
> PageCloneSpecial - copy special section from source page
> PageRestoreTempPage - no change.

That naming still breaks the association of "TempPage" functions.
If we're going to have multiple temp-page-creation functions,
I think their names should follow a pattern like PageGetTempPageXXX.

After looking around a bit, I'm not entirely convinced that there's
*any* call for the existing definition of PageGetTempPage :-(.
There are only two callers: _bt_split() which certainly doesn't need
it to work the way it does, and gistplacetopage() which might or
might not be just as happy initializing all of the page special
space for itself.  Oleg, Teodor, could you comment on whether it's
really needed to copy the old page's special space there?

Also, to the extent that PageGetTempPage copies the source page's
header instead of setting it up from scratch, I think it's outright
*wrong*.  This will result in copying the source's pd_flags and
pd_prune_xid, neither of which seems like correct behavior given that
we're clearing the page contents.

I'm thinking we should split PageGetTempPage into two versions:

PageGetTempPage: get a temp page the same size as the given page,
but don't initialize its contents at all (so, just a thin wrapper
for palloc).  This could be used by _bt_split, as well as
GinPageGetCopyPage and GistPageGetCopyPage.

PageGetTempPageCopySpecial: get a temp page, PageInit it, and
copy the special space from the given page.  The only customer
for this is gistplacetopage(), so maybe we don't even want it,
rather than just doing the work right in gistplacetopage()?

You could also make an argument for PageGetTempPageCopy() which'd just
copy the source page verbatim, thus replacing GinPageGetCopyPage and
GistPageGetCopyPage.
        regards, tom lane