Re: _RollbackFunc : dead code?

While trying to grok heap_update I came again across the
_heap_unlock_tuple function. This code apparently tries to save a XLog
round while trying to mark a tuple for update, by registering a
"rollback callback", which would unmark the tuple in case the
transaction is rolled back.

Turns out the callback is never called at all. So the code is dead
code.

Also, it claims that by marking t_infomask with a special
HEAP_XMAX_UNLOGGED bit, this trick would not suffer across a system
crash, because tqual routines would check this bit (comments in the code
don't specify what would the routines do with it).

Turns out tqual routines never check the bit. In fact, the bit is never
checked at all, nowhere in the code: heap_update sets it and then
unsets it, but that's all.

So I think this is dead code. The attached patch removes it.

--
Alvaro Herrera (<alvherre[(at)]dcc(dot)uchile(dot)cl>)
Maybe there's lots of data loss but the records of data loss are also lost.
(Lincoln Yeoh)

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> So I think this is dead code. The attached patch removes it.

Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
proceed in that direction, so we may as well remove the last traces.

regards, tom lane

On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > So I think this is dead code. The attached patch removes it.
>
> Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
> Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
> proceed in that direction, so we may as well remove the last traces.

Agreed.

We still need to explain *why* at some point, but thats still one of my
WIPs.

Best Regards, Simon Riggs

On Thu, Mar 31, 2005 at 05:07:39PM +0100, Simon Riggs wrote:
> On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:
> > Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > > So I think this is dead code. The attached patch removes it.
> >
> > Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
> > Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
> > proceed in that direction, so we may as well remove the last traces.
>
> Agreed.
>
> We still need to explain *why* at some point, but thats still one of my
> WIPs.

Sorry, what's your WIP? Explain why nobody wants to implement UNDO? Or
implement UNDO? Or why at some point somebody wanted to implement UNDO?

Now I remember that in the WAL docs there is a paragraph or two
mentioning that in a future project we want to implement UNDO ... maybe
it's a good idea to rip that off.

--
Alvaro Herrera (<alvherre[(at)]dcc(dot)uchile(dot)cl>)
"Un poeta es un mundo encerrado en un hombre" (Victor Hugo)

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> Now I remember that in the WAL docs there is a paragraph or two
> mentioning that in a future project we want to implement UNDO ... maybe
> it's a good idea to rip that off.

It is gone as of 8.0.

regards, tom lane

On Thu, 2005-03-31 at 12:24 -0400, Alvaro Herrera wrote:
> On Thu, Mar 31, 2005 at 05:07:39PM +0100, Simon Riggs wrote:
> > On Sun, 2005-03-27 at 16:01 -0500, Tom Lane wrote:
> > > Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> > > > So I think this is dead code. The attached patch removes it.
> > >
> > > Yeah, it is dead code; it's a leftover from Vadim's old plan to implement
> > > Oracle-style UNDO. AFAIK none of the current crop of hackers wants to
> > > proceed in that direction, so we may as well remove the last traces.
> >
> > Agreed.
> >
> > We still need to explain *why* at some point, but thats still one of my
> > WIPs.
>
> Sorry, what's your WIP? Explain why nobody wants to implement UNDO? Or
> implement UNDO? Or why at some point somebody wanted to implement UNDO?

Nearly: explain why UNDO is not required, and need not be implemented.

Anyway, its a WIP, but not unfortunately a high one, right now.

> Now I remember that in the WAL docs there is a paragraph or two
> mentioning that in a future project we want to implement UNDO ... maybe
> it's a good idea to rip that off.

I agree.

Best Regards, Simon Riggs