RE: parallel vacuum comments

On Wed, Dec 22, 2021 11:36 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Tue, Dec 21, 2021 at 10:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Tue, Dec 21, 2021 at 11:24 AM Masahiko Sawada
> <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Dec 21, 2021 at 2:04 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > > >
> > >
> > > Thank you for the comment. Agreed.
> > >
> > > I've attached updated version patches. Please review them.
> > >
> >
> > These look mostly good to me. Please find attached the slightly edited
> > version of the 0001 patch. I have modified comments, ran pgindent, and
> > modify the commit message. I'll commit this tomorrow if you are fine
> > with it.
>
> Thank you for committing the first patch.
>
> I've attached an updated version second patch. Please review it.
>
> Regards,

Hi,

The patch looks mostly good to me.
I only have few comments.

1)
+/*
+ * Do parallel index bulk-deletion with parallel workers.
+ */
+void
+parallel_vacuum_bulkdel_all_indexes(ParallelVacuumState *pvs, long num_table_tuples)
+{
+ Assert(!IsParallelWorker());
+

Would it be better to also put Assert(pvs != NULL) here ? Because we removed
the Assert(ParallelVacuumIsActive(vacrel)) check in the old function.

2)
+#include "utils/rel.h"
+#include "utils/lsyscache.h"
+#include "utils/memutils.h"

It might be better to keep the header file in alphabetical order.
:
+#include "utils/lsyscache.h"
+#include "utils/memutils.h"
+#include "utils/rel.h"

Best regards,
Hou zj