Hi,
I was interested in this feature, and took an initial look through the patch. Sorry in advance that I'm missing some previous context from the thread's history, I'm starting fresh here.
A few initial notes from looking at the v34 patches:
Usability and docs:
* We're leaving behind two invalid indexes now that the user has to figure out how to drop in case of an error - that seems like it could be confusing for the user. Could we have some better way (error handler, background worker) try to perform this cleanup automatically? If not, we should at least tell the user clearly in the error message that both invalid indexes are left behind (i.e. "idx" and "idx_ccaux" in the example)
* Docs are inconsistent or confusing about whether there's one or two indexes left behind in case of error - e.g. "command will fail but leave behind *an* invalid index and its associated auxiliary index" - somewhat implying there is only one invalid index, and somehow the auxiliary index is valid?
* Similarly, when the doc mentions e.g. "drop the index" - it's not necessarily clear which index we're talking about when there are two invalid indexes left behind that the user will see with `\d`
* It would be nice to guard against users trying arbitrary CREATE INDEX ... USING stir(...) with a clear error
Few behavior notes and questions:
* One of the testcases (line 2478 of patch 0004) does `DELETE FROM concur_reindex_tab4 WHERE c1 = 1;` but the table `concur_reindex_tab4` looks like it has been dropped a few lines above that?
* The StirPageGetFreeSpace macro from patch 0002 reads `StirPageGetMaxOffset(page)` which seems like it could cause an unsafe read of opaque->maxoff if used on the metapage
* A comment explains "No predicate evaluation is needed here" , i.e. we are skipping predicate evaluation in the validation scan step, assuming that the auxiliary index contains only qualifying TIDs. Is this really bulletproof for e.g. partial indexes which may no longer satisfy the predicate at the time of the validation scan due to conflicting HOT updates?
Thanks
Josh
