Yamada-san,
Thank you for updating the patch.
On Wed, Nov 27, 2019 at 12:46 PM Tatsuro Yamada
<tatsuro.yamada.tf@nttcom.co.jp> wrote:
> But I just remembered I replaced column name "*_table" with "*_relid"
> based on Robert's comment three months ago, see below:
>
> > /me reviews.
> >
> > + <entry><structfield>scanning_table</structfield></entry>
> >
> > I think this should be retitled to something that ends in 'relid',
> > like all of the corresponding cases in existing progress views.
> > Perhaps 'active_relid' or 'current_relid'.
>
> So, it would be better to use same rule against child_tables_total and
> child_tables_done. Thus I changed these column names to others and added
> to the view.
Robert's comment seems OK for a column that actually reports an OID,
but for columns that report counts, names containing "relids" sound a
bit strange to me. So, I prefer child_tables_total /
child_tables_done over child_relids_total / child_relids_done. Would
like to hear more opinions.
> >> Also, inheritance tree stats are created *after* creating single table
> >> stats, so I think that it would be better to have a distinct phase
> >> name for that, say "acquiring inherited sample rows". In
> >> do_analyze_rel(), you can select which of two phases to set based on
> >> whether inh is true or not.
> >
> > Okay! I'll also add the new phase "acquiring inherited sample rows" on
> > the next patch.
>
>
> Fixed.
>
> I tried to abbreviate it to "acquiring inh sample rows" because I thought
> "acquiring inherited sample rows" is a little long for the phase name.
I think phase names should contain full English words, because they
are supposed to be descriptive. Users are very likely to not
understand what "inh" means without looking up the docs.
Thanks,
Amit
