On Sat, Mar 20, 2021 at 01:06:51PM +0900, Masahiko Sawada wrote: > It's not bad but it seems redundant a bit to me. We pass the idx in > spite of passing also Irel[idx] and &(vacrelstats->indstats[idx]). I > think your first idea that is done in v4 patch (saving index names at > the beginning of heap_vacuum_rel() for autovacuum logging purpose > only) and the idea of deferring to close indexes until the end of > heap_vacuum_rel() so that we can refer index name at autovacuum > logging are more simple.
Okay. > We need to initialize *stats with NULL here. Right. I am wondering why I did not get any complain here. > If shared_indstats is NULL (e.g., we do " stats = > vacrelstats->indstats[indnum];"), vacrelstats->indstats[indnum] is not > updated since we pass &stats. I think we should pass > &(vacrelstats->indstats[indnum]) instead in this case. If we get rid completely of this idea around indnum, that I don't disagree with so let's keep just indname, you mean to keep the second argument IndexBulkDeleteResult of vacuum_one_index() and pass down &(vacrelstats->indstats[indnum]) as argument. No objections from me to just do that. > Previously, we update the element of the pointer array of index > statistics to the pointer pointing to either the local memory or DSM. > But with the above change, we do that only when the index statistics > are in the local memory. In other words, vacrelstats->indstats[i] is > never updated if the corresponding index supports parallel indexes. I > think this is not relevant with the change that we'd like to do here > (i.e., passing indnum down). Yeah, that looks like just some over-engineering design on my side. Would you like to update the patch with what you think is most adapted? -- Michael
signature.asc
Description: PGP signature
