Re: Unlogged tables cannot be truncated twice

On Thursday, June 02, 2011 07:31:33 PM Robert Haas wrote:
> On Wed, Jun 1, 2011 at 1:15 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > 2011/5/31 Andres Freund <andres(at)anarazel(dot)de>:
> >> On Tuesday, May 31, 2011 03:27:22 Alvaro Herrera wrote:
> >>> Excerpts from Andres Freund's message of lun may 30 20:47:49 -0400 2011:
> >>> > On Tuesday, May 31, 2011 02:35:58 AM Andres Freund wrote:
> >>> > > On Tuesday, May 31, 2011 02:14:00 AM Andres Freund wrote:
> >>> > > > On Tuesday, May 31, 2011 01:56:05 AM Cédric Villemain wrote:
> >>> > > > > I remove my own explanations as we conclude on the same thing.
> >>> > > > > Attached is the fix by adding a (!reindex) in the index.c
> >>> > > > > if().
> >>> > > >
> >>> > > > Thats imo wrong because it will break a plain REINDEX?
> >>> > >
> >>> > > > I think one possible correct fix would be the attached:
> >>> > > My version was wrong as well because it did not observe
> >>> > > RelationTruncate's nblocks argument. That function is used to
> >>> > > "shorten" the relation in vacuum. So dropping the init fork there
> >>> > > is not a good idea.
> >>> > >
> >>> > > So I think it is the simpler version of simply checking the
> >>> > > existance of the fork before creating is ok.
> >>>
> >>> Hmm, I wonder if what we should be doing here is observe isreindex in
> >>> index_build to avoid creating the init fork. Doing smgr access at that
> >>> level seems wrong.
> >>
> >> isreindex doesn't contain the necessary informormation as its set doing
> >> a REINDEX even though a new relfilenode is created and thus the fork
> >> needs to be created.
> >>
> >> It doesn't seem terribly clean do do the !smgrexists(), I aggree with
> >> you there. On the other hand we are calling smgrcreate() two lines down
> >> anyway. I personally don't realy like the placement of that piece of
> >> code very much. Doing it in index_build seems to be the wrong place. I
> >> don't think there really is a good place for it right now.
> >
> > I'm open to suggestions on how to rearrange this, but I think for
> > right now the approach you proposed upthread (add a smgrexists() test)
> > is probably the simplest way to fix this.
>
> Done. Your patch tested for FSM_FORKNUM instead of INIT_FORKNUM,
> which seemed wrong, so I changed it. I also added comments.
Wow. I don't think I ever made so many stupid mistakes when doing a two line
change. I abviously wasn't really awake that evening. As evidenced excessively
in that thread ;)

Thanks,

Andres