Re: REINDEX CONCURRENTLY unexpectedly fails

Hi,

On 2019-11-13 15:29:53 +0100, Manuel Rigger wrote:
> On the latest trunk version, I get an error "index "t0_pkey_ccnew"
> already contains data" when using REINDEX CONCURRENTLY:
>
> CREATE TEMP TABLE t0(c1 INT PRIMARY KEY) ON COMMIT DELETE ROWS;
> REINDEX TABLE CONCURRENTLY t0; -- unexpected: ERROR:  index
> "t0_pkey_ccnew" already contains data
>
> Is this expected? I think I did not observe this error on earlier
> PostgreSQL versions.

That seems pretty clearly a bug.

The problem is that the CONCURRENTLY code executes the ON COMMIT action
during CIC's internal transactions. Which then pretty completely breaks
the REINDEX operation. I think there's also a clear lack of error
checking about the index still being the correct one in the CIC code
(not recent), and I think we also need more error checking for the
truncate code (something CheckTableNotInUse() like).

The trace:
#0  index_build (heapRelation=0x7f006d49b998, indexRelation=0x7f006d499b80, indexInfo=0x55a46121b858, isreindex=true,
parallel=false)
    at /home/andres/src/postgresql/src/backend/catalog/index.c:2758
#1  0x000055a45fd43853 in RelationTruncateIndexes (heapRelation=0x7f006d49b998) at
/home/andres/src/postgresql/src/backend/catalog/heap.c:3161
#2  0x000055a45fd43b86 in heap_truncate_one_rel (rel=0x7f006d49b998) at
/home/andres/src/postgresql/src/backend/catalog/heap.c:3234
#3  0x000055a45fd43a6d in heap_truncate (relids=0x55a46121b820) at
/home/andres/src/postgresql/src/backend/catalog/heap.c:3202
#4  0x000055a45ff337cb in PreCommit_on_commit_actions () at
/home/andres/src/postgresql/src/backend/commands/tablecmds.c:14652
#5  0x000055a45fcd7258 in CommitTransaction () at /home/andres/src/postgresql/src/backend/access/transam/xact.c:2110
#6  0x000055a45fcd8e80 in CommitTransactionCommand () at
/home/andres/src/postgresql/src/backend/access/transam/xact.c:2923
#7  0x000055a45fecb790 in ReindexRelationConcurrently (relationOid=16409, options=0) at
/home/andres/src/postgresql/src/backend/commands/indexcmds.c:3035
#8  0x000055a45fec9380 in ReindexTable (relation=0x55a461084858, options=0, concurrent=true)
    at /home/andres/src/postgresql/src/backend/commands/indexcmds.c:2447

*ponders*

This probably is triggerable before v12 as well. Not with REINDEX
CONCURRENTLY, but with CREATE INDEX CONCURRENTLY.

Indeed:

postgres[7782][1]=# CREATE TEMP TABLE t0(c1 INT PRIMARY KEY) ON COMMIT DELETE ROWS;
CREATE TABLE

postgres[7782][1]=# CREATE INDEX CONCURRENTLY t0_c1 ON t0(c1);
ERROR:  XX000: index "t0_c1" already contains data
LOCATION:  btbuild, nbtsort.c:321

postgres[7782][1]=# SELECT version();
┌──────────────────────────────────────────────────────────────────────────────────────────────────┐
│                                             version                                              │
├──────────────────────────────────────────────────────────────────────────────────────────────────┤
│ PostgreSQL 11.5 on x86_64-pc-linux-gnu, compiled by gcc (Debian 9.2.1-15) 9.2.1 20191027, 64-bit │
└──────────────────────────────────────────────────────────────────────────────────────────────────┘
(1 row)


I think this quite possibly has been broken since CIC's introduction.


It think we really ought to just refuse CIC (and thereby REINDEX
CONCURRENTLY) for ON COMMIT DELETE/DROP temp tables. Given that CIC
internally uses transactions, it makes no sense to use CIC on such a
table.


Greetings,

Andres Freund