Hi Ashutosh,
On 2017/03/19 17:56, Ashutosh Sharma wrote:
> Hi,
>
> I didn't find any major issues with the patch. It works as expected.
> However, I have few minor comments that I would like to share,
>
> + <entry>
> + Total number of sample rows. The sample it reads is taken randomly.
> + Its size depends on the <varname>default_statistics_target</>
> parameter value.
> + </entry>
>
> 1) I think it would be better if you could specify reference link to
> the guc variable 'default_statistics_target'. Something like this,
>
> <xref linkend='guc-default-statistics-target'>.
>
> If you go through monitoring.sgml, you would find that there is
> reference link to all guc variables being used.
+1. Updated in the attached patch.
>
> 2) I feel that the 'computing_statistics' phase could have been
> splitted into 'computing_column_stats', 'computing_index_stats'....
> Please let me know your thoughts on this.
>
Initially I have spitted this phase as 'computing heap stats' and
'computing index stats' but
I agreed with Roberts suggestion to merge two phases into one as
'computing statistics'.
> + certain commands during command execution. Currently, the command
> + which supports progress reporting is <command>VACUUM</> and
> <command>ANALYZE</>. This may be
> expanded in the future.
>
> 3) I think above needs to be rephrased. Something like...Currently,
> the supported progress reporting commands are 'VACUUM' and
> 'ANALYZE'.
+1. Updated in the attached patch.
> Moreover, I also ran your patch on Windows platform and didn't find
> any issues. Thanks.
Thank you for testing the patch on Windows platform.
Regards,
Vinayak Pokale
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
