Thread: Representation of redirected line pointers in HOT

I find the HOT patch's representation of redirected line pointers pretty
klugy.  It's got a magic offset number to mean one thing, and a magic
length number to mean something else, and the assumption that either of
these can't correspond to a real offset or length seems pretty weak.
(It would fail if we could have one-byte tuples, which of course is
nowhere near reality, but still...)

What I'm thinking is that we should instead do this by extending the use
of the lp_flags field.  lp_flags is two bits, which we currently define
as independent LP_USED and LP_DELETE bits, but in fact LP_DELETE is
never used in heap pages.  (It is used in indexes.)  I propose that
we redefine lp_flags as having four states, say
LP_UNUSED    0LP_NORMAL    1LP_REDIRECT    2LP_DEAD        3

The LP_DEAD state would have slightly different meanings in indexes and
heap pages: in an index this would represent an entry that is known dead
but hasn't been deleted, whereas in a heap page this would correspond to
what the HOT patch calls a "redirect dead" line pointer, that is one
that has no associated tuple storage but can't be removed because index
entries still link to it.  We could make that difference explicit in the
line pointer, though: if it still has storage then lp_offset and lp_len
point to that storage, and if it doesn't have storage then they are set
to zero.

UNUSED pointers would also have offset = len = 0, and REDIRECT pointers
would have offset = link to next line pointer and len = 0, leading to
the general rule that "if it's got storage, len > 0, otherwise len = 0".

The above state values are chosen with malice aforethought to match the
bit patterns for the LP_USED/LP_DELETE combinations that are actually in
use today, so this change is upward compatible, except that I'm not sure
how careful we are about setting len = 0 in unused line pointers.

This seems hardly any uglier than the way the code stands today, and
certainly a lot less ugly than what the current HOT patch proposes.

Comments?
        regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

>     LP_UNUSED    0
>     LP_NORMAL    1
>     LP_REDIRECT    2
>     LP_DEAD        3

> This seems hardly any uglier than the way the code stands today, and
> certainly a lot less ugly than what the current HOT patch proposes.
>
> Comments?

If I understand correctly this still leaves open the possibility of
implementing in the future "quick pruning" as we've been speculating about. We
could represent that with a line pointer which is LP_DEAD but still has a
length and offset. I'm not sure we need to do it now but I'll be glad if we
aren't foreclosing the possibility.

These kinds of rethinks are typical of the tension between someone writing a
patch to submit for review, where they often want to keep the lines of code
changed to a minimum to avoid conflicts and to avoid giving reviewers extra
code to read which, and normal code maintenance. 


--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com

Gregory Stark wrote:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
> 
> >     LP_UNUSED    0
> >     LP_NORMAL    1
> >     LP_REDIRECT    2
> >     LP_DEAD        3
> 
> > This seems hardly any uglier than the way the code stands today, and
> > certainly a lot less ugly than what the current HOT patch proposes.
> >
> > Comments?
> 
> If I understand correctly this still leaves open the possibility of
> implementing in the future "quick pruning" as we've been speculating about. We
> could represent that with a line pointer which is LP_DEAD but still has a
> length and offset. I'm not sure we need to do it now but I'll be glad if we
> aren't foreclosing the possibility.
> 
> These kinds of rethinks are typical of the tension between someone writing a
> patch to submit for review, where they often want to keep the lines of code
> changed to a minimum to avoid conflicts and to avoid giving reviewers extra
> code to read which, and normal code maintenance. 

Yes, good point.  This is why I am glad Tom can give it a full review.

--  Bruce Momjian  <bruce@momjian.us>          http://momjian.us EnterpriseDB
http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Gregory Stark <stark@enterprisedb.com> writes:
> If I understand correctly this still leaves open the possibility of
> implementing in the future "quick pruning" as we've been speculating about. We
> could represent that with a line pointer which is LP_DEAD but still has a
> length and offset. I'm not sure we need to do it now but I'll be glad if we
> aren't foreclosing the possibility.

You could still do that, but I'm not sure I see the point.
        regards, tom lane

Sounds good to me. I saw you committed this change. Do you want me
to update HOT patch to use this or you are already doing that ?


Thanks,
Pavan

--
Pavan Deolasee
EnterpriseDB     http://www.enterprisedb.com

"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> Sounds good to me. I saw you committed this change. Do you want me
> to update HOT patch to use this or you are already doing that ?

I'll clean up my own mess ;-)
        regards, tom lane