Skip to content
Snippets Groups Projects
  • Tom Lane's avatar
    94c014b5
    Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY. · 94c014b5
    Tom Lane authored
    Commit 8cb53654, which introduced DROP
    INDEX CONCURRENTLY, managed to break CREATE INDEX CONCURRENTLY via a poor
    choice of catalog state representation.  The pg_index state for an index
    that's reached the final pre-drop stage was the same as the state for an
    index just created by CREATE INDEX CONCURRENTLY.  This meant that the
    (necessary) change to make RelationGetIndexList ignore about-to-die indexes
    also made it ignore freshly-created indexes; which is catastrophic because
    the latter do need to be considered in HOT-safety decisions.  Failure to
    do so leads to incorrect index entries and subsequently wrong results from
    queries depending on the concurrently-created index.
    
    To fix, make the final state be indisvalid = true and indisready = false,
    which is otherwise nonsensical.  This is pretty ugly but we can't add
    another column without forcing initdb, and it's too late for that in 9.2.
    (There's a cleaner fix in HEAD.)
    
    In addition, change CREATE/DROP INDEX CONCURRENTLY so that the pg_index
    flag changes they make without exclusive lock on the index are made via
    heap_inplace_update() rather than a normal transactional update.  The
    latter is not very safe because moving the pg_index tuple could result in
    concurrent SnapshotNow scans finding it twice or not at all, thus possibly
    resulting in index corruption.  This is a pre-existing bug in CREATE INDEX
    CONCURRENTLY, which was copied into the DROP code.
    
    In addition, fix various places in the code that ought to check to make
    sure that the indexes they are manipulating are valid and/or ready as
    appropriate.  These represent bugs that have existed since 8.2, since
    a failed CREATE INDEX CONCURRENTLY could leave a corrupt or invalid
    index behind, and we ought not try to do anything that might fail with
    such an index.
    
    Also fix RelationReloadIndexInfo to ensure it copies all the pg_index
    columns that are allowed to change after initial creation.  Previously we
    could have been left with stale values of some fields in an index relcache
    entry.  It's not clear whether this actually had any user-visible
    consequences, but it's at least a bug waiting to happen.
    
    In addition, do some code and docs review for DROP INDEX CONCURRENTLY;
    some cosmetic code cleanup but mostly addition and revision of comments.
    
    Portions of this need to be back-patched even further, but I'll work
    on that separately.
    
    Problem reported by Amit Kapila, diagnosis by Pavan Deolasee,
    fix by Tom Lane and Andres Freund.
    94c014b5
    History
    Fix assorted bugs in CREATE/DROP INDEX CONCURRENTLY.
    Tom Lane authored
    Commit 8cb53654, which introduced DROP
    INDEX CONCURRENTLY, managed to break CREATE INDEX CONCURRENTLY via a poor
    choice of catalog state representation.  The pg_index state for an index
    that's reached the final pre-drop stage was the same as the state for an
    index just created by CREATE INDEX CONCURRENTLY.  This meant that the
    (necessary) change to make RelationGetIndexList ignore about-to-die indexes
    also made it ignore freshly-created indexes; which is catastrophic because
    the latter do need to be considered in HOT-safety decisions.  Failure to
    do so leads to incorrect index entries and subsequently wrong results from
    queries depending on the concurrently-created index.
    
    To fix, make the final state be indisvalid = true and indisready = false,
    which is otherwise nonsensical.  This is pretty ugly but we can't add
    another column without forcing initdb, and it's too late for that in 9.2.
    (There's a cleaner fix in HEAD.)
    
    In addition, change CREATE/DROP INDEX CONCURRENTLY so that the pg_index
    flag changes they make without exclusive lock on the index are made via
    heap_inplace_update() rather than a normal transactional update.  The
    latter is not very safe because moving the pg_index tuple could result in
    concurrent SnapshotNow scans finding it twice or not at all, thus possibly
    resulting in index corruption.  This is a pre-existing bug in CREATE INDEX
    CONCURRENTLY, which was copied into the DROP code.
    
    In addition, fix various places in the code that ought to check to make
    sure that the indexes they are manipulating are valid and/or ready as
    appropriate.  These represent bugs that have existed since 8.2, since
    a failed CREATE INDEX CONCURRENTLY could leave a corrupt or invalid
    index behind, and we ought not try to do anything that might fail with
    such an index.
    
    Also fix RelationReloadIndexInfo to ensure it copies all the pg_index
    columns that are allowed to change after initial creation.  Previously we
    could have been left with stale values of some fields in an index relcache
    entry.  It's not clear whether this actually had any user-visible
    consequences, but it's at least a bug waiting to happen.
    
    In addition, do some code and docs review for DROP INDEX CONCURRENTLY;
    some cosmetic code cleanup but mostly addition and revision of comments.
    
    Portions of this need to be back-patched even further, but I'll work
    on that separately.
    
    Problem reported by Amit Kapila, diagnosis by Pavan Deolasee,
    fix by Tom Lane and Andres Freund.