Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY
Date
Msg-id CALNJ-vQf3kESrdgyvS=B6gbggu+W3Q++KQObnEkYbNd0qGnQgA@mail.gmail.com
Whole thread Raw
In response to Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY  (Zhihong Yu <zyu@yugabyte.com>)
Responses Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers


On Sat, Oct 30, 2021 at 3:59 AM Zhihong Yu <zyu@yugabyte.com> wrote:


On Sat, Oct 30, 2021 at 1:28 AM Michael Paquier <michael@paquier.xyz> wrote:
Hi all,

While reviewing the code for opclass parameters with indexes, I have
noticed that opclass parameters are lost after a concurrent reindex.
As we use a IndexInfo to hold the information of the new index when
creating a copy of the old one, it is just a matter of making sure
that ii_OpclassOptions is filled appropriately, but that was missed by
911e702.

Attached is a patch to fix the issue.  After a concurrent reindex, we
would not finish with a corrupted index, just with one rebuilt with
default opclass parameter values.

Any objections or comments?
--
Michael
Hi,

+       newInfo->ii_OpclassOptions = palloc0(sizeof(Datum) *
+                                            newInfo->ii_NumIndexAttrs);

It seems we may not need to allocate these many entries (as shown by the concur_appclass_ind_2 example in the test). 
In the previous loop (starting line 1359), we can increment a counter which would finally tell us how many oldInfo->ii_OpclassOptions[i] is not NULL.

Then that many entries can be allocated in the above code.

Cheers
Hi,
Upon further look, my previous comment was premature. Please ignore that.

+           newInfo->ii_OpclassOptions[i] = oldInfo->ii_OpclassOptions[i];

Should datumCopy() be used inside the loop ? I saw the following in get_attoptions(Oid relid, int16 attnum):

        result = datumCopy(attopts, false, -1); /* text[] */ 

Cheers

pgsql-hackers by date:

Previous
From: Zhihong Yu
Date:
Subject: Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY
Next
From: Peter Eisentraut
Date:
Subject: Re: remove internal support in pgcrypto?