Re: REINDEX CONCURRENTLY 2.0

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: REINDEX CONCURRENTLY 2.0
Date: 2017-03-13 02:11:50
Message-ID: 08fdfd95-c0c7-681a-33de-f582fe480c26@proxel.se
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 03/02/2017 03:10 AM, Michael Paquier wrote:
> On Wed, Mar 1, 2017 at 2:21 AM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
> + /*
> + * Copy contraint flags for old index. This is safe because the old index
> + * guaranteed uniquness.
> + */
> + newIndexForm->indisprimary = oldIndexForm->indisprimary;
> + oldIndexForm->indisprimary = false;
> + newIndexForm->indisexclusion = oldIndexForm->indisexclusion;
> + oldIndexForm->indisexclusion = false;
> [...]
> + deleteDependencyRecordsForClass(RelationRelationId, newIndexOid,
> + RelationRelationId, DEPENDENCY_AUTO);
> + deleteDependencyRecordsForClass(RelationRelationId, oldIndexOid,
> + ConstraintRelationId,
> DEPENDENCY_INTERNAL);
> +
> + // TODO: pg_depend for old index?

Spotted one of my TODO comments there so I have attached a patch where I
have cleaned up that function. I also fixed the the code to properly
support triggers.

> There is a lot of mumbo-jumbo in the patch to create the exact same
> index definition as the original one being reindexed, and that's a
> huge maintenance burden for the future. You can blame me for that in
> the current patch. I am wondering if it would not just be better to
> generate a CREATE INDEX query string and then use the SPI to create
> the index, and also do the following extensions at SQL level:
> - Add a sort of WITH NO DATA clause where the index is created, so the
> index is created empty, and is marked invalid and not ready.
> - Extend pg_get_indexdef_string() with an optional parameter to
> enforce the index name to something else, most likely it should be
> extended with the WITH NO DATA/INVALID clause, which should just be a
> storage parameter by the way.
> By doing something like that what the REINDEX CONCURRENTLY code path
> should just be careful about is that it chooses an index name that
> avoids any conflicts.

Hm, I am not sure how much that would help since a lot of the mumb-jumbo
is by necessity in the step where we move the constraints over from the
old index to the new.

Andreas

Attachment Content-Type Size
reindex-concurrently-v2.patch text/x-patch 92.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-03-13 02:20:32 Re: make async slave to wait for lsn to be replayed
Previous Message David Rowley 2017-03-13 01:50:53 Re: [NOVICE] Why is there a doubtful copyObject call in add_vars_to_targetlist