Re: Support for REINDEX CONCURRENTLY

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: Support for REINDEX CONCURRENTLY
Date: 2013-03-28 01:18:45
Message-ID: CAB7nPqSTz=0iXzKvsqazz7Zto-myR_G8vmxZqPRj0J6z15++Fg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the comments. Please find updated patches attached.

On Thu, Mar 28, 2013 at 3:12 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> - reltoastidxid = rel->rd_rel->reltoastidxid;
> + /* Fetch the list of indexes on toast relation if necessary */
> + if (OidIsValid(reltoastrelid))
> + {
> + Relation toastRel = relation_open(reltoastrelid, lockmode);
> + RelationGetIndexList(toastRel);
> + reltoastidxids = list_copy(toastRel->rd_indexlist);
> + relation_close(toastRel, NoLock);
>
> list_copy() seems not to be required here. We can just set reltoastidxids
> to
> the return list of RelationGetIndexList().
>
Good catch. I thought that I took care of such things in previous versions
at
all the places.

Since we call relation_open() with lockmode, ISTM that we should also call
> relation_close() with the same lockmode instead of NoLock. No?
>
Agreed on that.

>
> - if (OidIsValid(reltoastidxid))
> - ATExecSetTableSpace(reltoastidxid, newTableSpace,
> lockmode);
> + foreach(lc, reltoastidxids)
> + {
> + Oid idxid = lfirst_oid(lc);
> + if (OidIsValid(idxid))
> + ATExecSetTableSpace(idxid, newTableSpace,
> lockmode);
>
> Since idxid is the pg_index.indexrelid, ISTM it should never be invalid.
> If this is true, the check of OidIsValid(idxid) is not required.
>
Indeed...
--
Michael

Attachment Content-Type Size
20130328_1_toastindex_v8.patch application/octet-stream 43.0 KB
20130328_2_reindex_concurrently_v25.patch application/octet-stream 81.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2013-03-28 01:34:06 Re: Support for REINDEX CONCURRENTLY
Previous Message Dean Rasheed 2013-03-28 01:16:18 Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)