Re: Vacuum statistics

Hi! Thank you for inquiring about our topic!

Hi!

Few suggestions on this patch-set

1)

+{ oid => '4701',
+  descr => 'pg_stats_vacuum_tables return stats values',
+  proname => 'pg_stats_vacuum_tables', provolatile => 's', prorettype => 'record',proisstrict => 'f',
+  proretset => 't',
+  proargtypes => 'oid oid',
+  proallargtypes =>

During development, OIDs should be picked up from range 8000-9999.
Same for pg_stats_vacuum_database & pg_stats_vacuum_indexes

Also, why are these function naming schemes like
pg_stats_vacuum_*something*, not pg_stat_vacuum_*something*, like
pg_stat_replication etc?

To be honest, when I named it, I missed this aspect. I thought about the plural vacuum statistics we show, so I named them. I fixed it.

2) In 0003:

+  proargnames => '{dboid,dboid,db_blks_read,db_blks_hit,total_blks_dirtied,total_blks_written,wal_records,wal_fpi,wal_bytes,blk_read_time,blk_write_time,delay_time,system_time,user_time,total_time,interrupts}',

Repeated dboid arg name is strange. Is it done this way to make
pg_stats_vacuum function call in more unified fashion? I don't see any
other place within postgresql core with similar approach, so I doubt
it is correct.

Both parameters are required for input and output. We are trying to find statistics for a specific database if the database oid was specified by the user or display statistics for all objects, but we need to display which database these statistics are for. I corrected the name of the first parameter.

3) 0001 patch vacuum_tables_statistics test creates
statistic_vacuum_database1, but does not use it. 0003 do.
Also I'm not sure if these additional checks on the second database
adds much value. Can you justify this please?

The statistic_vacuum_database1 needs us to check the visible of statistics from another database (statistic_vacuum_database) as they are after the manipulation with tables in another database, and after deleting the vestat table . In the latter case, we need to be sure that all the table statistics are not visible to us.

So, I agree that it should be added only in the latest version of the patch, where we add vacuum statistics for databases. I fixed it.

Other places look more or less fine to me.
However, I'll maybe post some additional nit-picky comments on the
next patch version.

We are glad any feedback and review, so feel free to do it)

-- 
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company