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-07-03 01:03:26 |
Message-ID: | CAB7nPqTeZpe84m38-+vEWrRHDtzacUzez033uBwDkpTi+Gnv-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Updated version of this patch attached. At the same time I changed
toastrel_valueid_exists back to its former shape by removing the extra
LOCKMODE argument I added to pass argument a lock to
toast_open_indexes and toast_close_indexes as at all the places only
RowExclusiveLock is used.
On Wed, Jul 3, 2013 at 5:51 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Wed, Jul 3, 2013 at 5:43 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Wed, Jul 3, 2013 at 5:22 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> Why did you remove the check of indisvalid from the --binary-upgrade SQL?
>>> Without this check, if there is the invalid toast index, more than one rows are
>>> returned and ExecuteSqlQueryForSingleRow() would cause the error.
>>>
>>> + foreach(lc, indexlist)
>>> + *toastidxs[i++] = index_open(lfirst_oid(lc), lock);
>>>
>>> *toastidxs[i++] should be (*toastidxs)[i++]. Otherwise, segmentation fault can
>>> happen.
>>>
>>> For now I've not found any other big problem except the above.
>
> system_views.sql
> - GROUP BY C.oid, N.nspname, C.relname, T.oid, X.oid;
> + GROUP BY C.oid, N.nspname, C.relname, T.oid, X.indexrelid;
>
> I found another problem. X.indexrelid should be X.indrelid. Otherwise, when
> there is the invalid toast index, more than one rows are returned for the same
> relation.
Indeed, fixed
>
>> OK cool, updated version attached. If you guys think that the attached
>> version is fine (only the reltoasyidxid removal part), perhaps it
>> would be worth committing it as Robert also committed the MVCC catalog
>> patch today. So we would be able to focus on the core feature asap
>> with the 2nd patch, and the removal of AccessExclusiveLock at swap
>> step.
>
> Yep, will do. Maybe today.
I also double-checked with gdb and the REINDEX CONCURRENTLY patch
applied on top of the attached patch that the new code paths
introduced in tuptoaster.c are fine.
Regards,
--
Michael
Attachment | Content-Type | Size |
---|---|---|
20130701_1_remove_reltoastidxid_v17.patch | application/octet-stream | 46.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | James Sewell | 2013-07-03 01:04:51 | Re: [PATCH] Add an ldapoption to disable chasing LDAP referrals |
Previous Message | Peter Eisentraut | 2013-07-03 00:42:28 | Re: updated emacs configuration |