Hi Amit-san,

On 2019/11/28 10:59, Amit Langote wrote:
Yamada-san,

Thank you for updating the patch.

On Wed, Nov 27, 2019 at 12:46 PM Tatsuro Yamada
<tatsuro.yamada...@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.

Hmmm... I understand your opinion but I'd like to get more opinions too. :)
Do you prefer these column names? See below:

<Columns of the view>
 pid
 datid
 datname
 relid
 phase
 sample_blks_total
 heap_blks_scanned
 ext_stats_total
 ext_stats_computed
 child_tables_total  <= Renamed: s/relid/table/
 child_tables_done   <= Renamed: s/relid/table/
 current_child_table <= Renamed: s/relid/table/



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.


Okay, I will fix it.
   s/acquiring inh sample rows/acquiring inherited sample rows/


Thanks,
Tatsuro Yamada




Reply via email to