Re: Support for REINDEX CONCURRENTLY

On Wed, Jul 3, 2013 at 11:16 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-07-03 10:03:26 +0900, Michael Paquier wrote:
>> index 9ee9ea2..23e0373 100644
>> --- a/src/bin/pg_dump/pg_dump.c
>> +++ b/src/bin/pg_dump/pg_dump.c
>> @@ -2778,10 +2778,9 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>       PQExpBuffer upgrade_query = createPQExpBuffer();
>>       PGresult   *upgrade_res;
>>       Oid                     pg_class_reltoastrelid;
>> -     Oid                     pg_class_reltoastidxid;
>>
>>       appendPQExpBuffer(upgrade_query,
>> -                                       "SELECT c.reltoastrelid, t.reltoastidxid "
>> +                                       "SELECT c.reltoastrelid "
>>                                         "FROM pg_catalog.pg_class c LEFT JOIN "
>>                                         "pg_catalog.pg_class t ON (c.reltoastrelid = t.oid) "
>>                                         "WHERE c.oid = '%u'::pg_catalog.oid;",
>> @@ -2790,7 +2789,6 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>       upgrade_res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
>>
>>       pg_class_reltoastrelid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "reltoastrelid")));
>> -     pg_class_reltoastidxid = atooid(PQgetvalue(upgrade_res, 0, PQfnumber(upgrade_res, "reltoastidxid")));
>>
>>       appendPQExpBuffer(upgrade_buffer,
>>                                  "\n-- For binary upgrade, must preserve pg_class oids\n");
>> @@ -2803,6 +2801,10 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>               /* only tables have toast tables, not indexes */
>>               if (OidIsValid(pg_class_reltoastrelid))
>>               {
>> +                     PQExpBuffer index_query = createPQExpBuffer();
>> +                     PGresult   *index_res;
>> +                     Oid                     indexrelid;
>> +
>>                       /*
>>                        * One complexity is that the table definition might not require
>>                        * the creation of a TOAST table, and the TOAST table might have
>> @@ -2816,10 +2818,23 @@ binary_upgrade_set_pg_class_oids(Archive *fout,
>>                                                         "SELECT binary_upgrade.set_next_toast_pg_class_oid('%u'::pg_catalog.oid);\n",
>>                                                         pg_class_reltoastrelid);
>>
>> -                     /* every toast table has an index */
>> +                     /* Every toast table has one valid index, so fetch it first... */
>> +                     appendPQExpBuffer(index_query,
>> +                                                       "SELECT c.indexrelid "
>> +                                                       "FROM pg_catalog.pg_index c "
>> +                                                       "WHERE c.indrelid = %u AND c.indisvalid;",
>> +                                                       pg_class_reltoastrelid);
>> +                     index_res = ExecuteSqlQueryForSingleRow(fout, index_query->data);
>> +                     indexrelid = atooid(PQgetvalue(index_res, 0,
>> +                                                                        PQfnumber(index_res, "indexrelid")));
>> +
>> +                     /* Then set it */
>>                       appendPQExpBuffer(upgrade_buffer,
>>                                                         "SELECT binary_upgrade.set_next_index_pg_class_oid('%u'::pg_catalog.oid);\n",
>> -                                                       pg_class_reltoastidxid);
>> +                                                       indexrelid);
>> +
>> +                     PQclear(index_res);
>> +                     destroyPQExpBuffer(index_query);
>
> Wouldn't it make more sense to fetch the toast index oid in the query
> ontop instead of making a query for every relation?
With something like a CASE condition in the upper query for
reltoastrelid? This code path is not only taken by indexes but also by
tables. So I thought that it was cleaner and more readable to fetch
the index OID only if necessary as a separate query.

Regards,
--
Michael