Re: btvacuumpage useless "orig_blkno"

Lists: pgsql-hackers
From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Subject: btvacuumpage useless "orig_blkno"
Date: 2011-11-21 22:03:59
Message-ID: 1321912069-sup-456@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I just noticed that btvacuumpage has two BlockNumber parameters -- blkno
and orig_blkno. The only caller passes them as the same value; the
header comments state that blkno would be different when recursing, but
actually the function implements recursion internally by way of a cute
"goto" trick. So it seems to me that the orig_blkno parameter is
useless -- we could just remove it.

Unless I'm completely missing something?

--
Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>

Attachment Content-Type Size
btvacuumscan.patch application/octet-stream 2.5 KB
From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Subject: Re: btvacuumpage useless "orig_blkno"
Date: 2011-11-21 22:11:21
Message-ID: CA+U5nMKJSbE-9qJO25QQ_6ns1ZEi=ww07eVm7bfPZQQRHngxFg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Nov 21, 2011 at 10:03 PM, Alvaro Herrera
<alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> I just noticed that btvacuumpage has two BlockNumber parameters -- blkno
> and orig_blkno.  The only caller passes them as the same value; the
> header comments state that blkno would be different when recursing, but
> actually the function implements recursion internally by way of a cute
> "goto" trick.  So it seems to me that the orig_blkno parameter is
> useless -- we could just remove it.
>
> Unless I'm completely missing something?

tail recursion - read comments at bottom of the function

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Subject: Re: btvacuumpage useless "orig_blkno"
Date: 2011-11-21 22:19:13
Message-ID: 1321913906-sup-7098@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Excerpts from Simon Riggs's message of lun nov 21 19:11:21 -0300 2011:
> On Mon, Nov 21, 2011 at 10:03 PM, Alvaro Herrera
> <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> > I just noticed that btvacuumpage has two BlockNumber parameters -- blkno
> > and orig_blkno.  The only caller passes them as the same value; the
> > header comments state that blkno would be different when recursing, but
> > actually the function implements recursion internally by way of a cute
> > "goto" trick.  So it seems to me that the orig_blkno parameter is
> > useless -- we could just remove it.
> >
> > Unless I'm completely missing something?
>
> tail recursion - read comments at bottom of the function

Right, but we don't need to pass the value as a parameter, we can just
save it at the start of the function, as my proposed patch does, right?

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Subject: Re: btvacuumpage useless "orig_blkno"
Date: 2011-11-22 02:14:58
Message-ID: 29399.1321928098@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Excerpts from Simon Riggs's message of lun nov 21 19:11:21 -0300 2011:
>> tail recursion - read comments at bottom of the function

> Right, but we don't need to pass the value as a parameter, we can just
> save it at the start of the function, as my proposed patch does, right?

If you do this, it's not really tail recursion anymore, so the comments
need to be adjusted. The patch sounds reasonable, but you have more
work to do to fix the comments ...

regards, tom lane