Re: progress report for ANALYZE

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.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.

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