[PATCH] PageGetTempPage cleanup

Started by Zdenek Kotalaover 17 years ago3 messageshackers
Jump to latest
#1Zdenek Kotala
Zdenek.Kotala@Sun.COM

I attach patch which cleans up code around PageGetTempPage. These changes were
discussed here:

http://archives.postgresql.org/pgsql-hackers/2008-08/msg00102.php

Zdenek

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

Attachments:

gettemppage.patchtext/x-diff; name=gettemppage.patchDownload+49-72
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zdenek Kotala (#1)
Re: [PATCH] PageGetTempPage cleanup

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:

I attach patch which cleans up code around PageGetTempPage. These changes were
discussed here:
http://archives.postgresql.org/pgsql-hackers/2008-08/msg00102.php

Applied with a minor change: instead of inventing
Page PageGetTempPage(Page page, bool copy)
I split it into two functions
Page PageGetTempPage(Page page)
Page PageGetTempPageCopy(Page page)
I don't see any advantage to having the single function, because it
doesn't seem like any calling code path would be likely to want both
behaviors depending on some condition. Moreover, the way you had it
meant that we'd be replacing
Page PageGetTempPage(Page page, Size specialSize);
with
Page PageGetTempPage(Page page, bool copy);
which seems risky to me. If someone failed to update code that was
meant to call the old API, they'd get no warning about it --- at least
not in any C compiler I'm familiar with. Changing the number of
arguments guarantees a compile error for un-updated code.

regards, tom lane

#3Zdenek Kotala
Zdenek.Kotala@Sun.COM
In reply to: Tom Lane (#2)
Re: [PATCH] PageGetTempPage cleanup

Tom Lane napsal(a):

Zdenek Kotala <Zdenek.Kotala@Sun.COM> writes:

I attach patch which cleans up code around PageGetTempPage. These changes were
discussed here:
http://archives.postgresql.org/pgsql-hackers/2008-08/msg00102.php

Applied with a minor change: instead of inventing
Page PageGetTempPage(Page page, bool copy)
I split it into two functions
Page PageGetTempPage(Page page)
Page PageGetTempPageCopy(Page page)
I don't see any advantage to having the single function, because it
doesn't seem like any calling code path would be likely to want both
behaviors depending on some condition. Moreover, the way you had it
meant that we'd be replacing
Page PageGetTempPage(Page page, Size specialSize);
with
Page PageGetTempPage(Page page, bool copy);
which seems risky to me. If someone failed to update code that was
meant to call the old API, they'd get no warning about it --- at least
not in any C compiler I'm familiar with. Changing the number of
arguments guarantees a compile error for un-updated code.

Agree.

Just a question you sometime argue that somebody should uses this interface for
external module. Is there any clue which function is used and which not? Should
we create something like core API description which will be stable?

Zdenek

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