thanks for the review!

> The comments do not reflect the function names ("total" is missing to give
> pg_stat_get_total_vacuum_time() and such).

fixed

> I think it's better to define the macro just before its first usage (meaning
> just after pg_stat_get_vacuum_count()): that would be consistent with the 
> places
> the other macros are defined.

correct. I fixed this to match the placement of similar macros.

> s/int64/double/?

fixed

> Mention the unit?

fixed

> Out of curiosity, why this extra comment? To be somehow consistent with
> do_analyze_rel()?

Yes, I added it for consistency, but since the comment was not there
before, I just decided to remove it.


> I wonder if it wouldn't make more sense to put the GetCurrentTimestamp() call
> before the pgstat_progress_start_command() call. That would be aligned with 
> the
> "endtime" being after the pgstat_progress_end_command() and where it was 
> before
> the patch.

I agree. Fixed.

Please see the attached v3.

Regards,

Sami

Attachment: v3-0001-Track-per-relation-cumulative-time-spent-in-vacuu.patch
Description: Binary data

Reply via email to