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.fujii@gmail.com> wrote:
> On Wed, Jul 3, 2013 at 5:43 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Wed, Jul 3, 2013 at 5:22 AM, Fujii Masao <masao.fujii@gmail.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