Re: Error with index on unlogged table

Hello,

At Thu, 10 Dec 2015 20:27:01 +0100, Andres Freund <andres(at)anarazel(dot)de> wrote in <20151210192701(dot)GC11331(at)alap3(dot)anarazel(dot)de>
> > > > A second problem is that the smgrimmedsync() in copy_relation_data()
> > > > isn't called for the init fork of unlogged relations, even if it needs
> > > > to.
>
> Here's a patch doing that. It's not yet fully polished, but I wanted to
> get it out, because I noticed one thing:
>
> In ATExecSetTableSpace(), for !main forks, we currently call
> smgrcreate(), but not log_smgrcreate(). Even for PERSISTENT
> relations. That seems a bit odd to me. It currently seems to be without
> further consequence because, if there's actual data in the fork, we'll
> just create the relation in _mdfd_getseg(); or we can cope with the
> relation not being there. But to me that feels wrong.

FWIW I agree that.

> It seems better to do the log_smgrcreate() for RELPERSISTENCE_PERMANENT,
> not just INIT_FORKNUM. What do you guys think?

What it is doing seems to me reasonable but copying_initfork
doesn't seems to be necessary. Kicking both of log_newpage() and
smgrimmedsync() by use_wal, which has the value considering
INIT_FORKNUM would be more descriptive. (more readable, in other
words)

> > It sounds worthwhile to check that other locations rewriting tables,
> > e.g. cluster/vacuum full/reindex are safe.
>
> Seems to be ok, on a first glance.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center