Re: hash index on unlogged tables doesn't behave as expected

On Thu, Jul 6, 2017 at 5:54 AM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Check for INIT_FORKNUM appears both accompanied and not
> accompanied by check for RELPER.._UNLOGGED, so I'm not sure which
> is the convention here.

Checking only for INIT_FORKNUM seems logical to me. Also checking for
RELPERSISTENCE_UNLOGGED just makes the code longer to no benefit. I
guess Amit copied the test from ATExecSetTablespace, which does it as
he did, but it seems unnecessarily long-winded.

> The difference of the two is an init fork of TEMP
> relations. However I believe that init fork is by definition a
> part of an unlogged relation, it seems to me that it ought not to
> be wal-logged if we had it. From this viewpoint, the middle line
> makes sense.

Actually, the init fork of an unlogged relation *must* be WAL-logged.
All other forks are removed on a crash (and the main fork is copied
anew from the init fork). But the init fork itself must survive -
therefore it, and only it, must be WAL-logged and thus durable.

> By the way the comment of the function ReadBufferWithoutRelcache
> has the following sentense.
>
> | * NB: At present, this function may only be used on permanent relations, which
> | * is OK, because we only use it during XLOG replay. If in the future we
> | * want to use it on temporary or unlogged relations, we could pass additional
> | * parameters.
>
> and does
>
> | return ReadBuffer_common(smgr, RELPERSISTENCE_PERMANENT, forkNum, blockNum,
> mode, strategy, &hit);
>
> This surely works since BufferAlloc recognizes INIT_FORK. But
> some adjustment may be needed around here.

Yeah, it should probably mention that the init fork of an unlogged
relation is also OK.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company