On Fri, Oct 18, 2019 at 4:51 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > I have prepared a first version of the patch. Currently, I am > performing an empty page deletion for all the cases. >
Few comments: ---------------------- 1. -/* - * State kept across vacuum stages. - */ typedef struct { - IndexBulkDeleteResult stats; /* must be first */ + IndexBulkDeleteResult *stats; /* kept across vacuum stages. */ /* - * These are used to memorize all internal and empty leaf pages in the 1st - * vacuum stage. They are used in the 2nd stage, to delete all the empty - * pages. + * These are used to memorize all internal and empty leaf pages. They are + * used for deleting all the empty pages. */ IntegerSet *internal_page_set; IntegerSet *empty_leaf_set; Now, if we don't want to share the remaining stats across gistbulkdelete and gistvacuumcleanup, isn't it better to keep the information of internal and empty leaf pages as part of GistVacState? Also, I think it is better to call gistvacuum_delete_empty_pages from function gistvacuumscan as that will avoid it calling from multiple places. 2. - gist_stats->page_set_context = NULL; - gist_stats->internal_page_set = NULL; - gist_stats->empty_leaf_set = NULL; Why have you removed this initialization? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com