Itagaki Takahiro escreveu: > The attached patch is rebased to current CVS. > I'm looking at your patch now... It is almost there but has some issues.
(i) documentation: you have more than three counters and they could be mentioned in docs too. + Include information on the buffers. Specifically, include the number of + buffer hits, number of disk blocks read, and number of local buffer read. (ii) format: why does text output format have less counters than the other ones? + if (es->format == EXPLAIN_FORMAT_TEXT) + { + appendStringInfoSpaces(es->str, es->indent * 2); + appendStringInfo(es->str, "Blocks Hit: %ld Read: %ld Temp Read: %ld\n", + usage->blks_hit, usage->blks_read, usage->temp_blks_read); + } + else + { + ExplainPropertyLong("Hit Blocks", usage->blks_hit, es); + ExplainPropertyLong("Read Blocks", usage->blks_read, es); + ExplainPropertyLong("Written Blocks", usage->blks_written, es); + ExplainPropertyLong("Local Hit Blocks", usage->local_blks_hit, es); + ExplainPropertyLong("Local Read Blocks", usage->local_blks_read, es); + ExplainPropertyLong("Local Written Blocks", usage->local_blks_written, es); + ExplainPropertyLong("Temp Read Blocks", usage->temp_blks_read, es); + ExplainPropertyLong("Temp Written Blocks", usage->temp_blks_written, es); + } (iii) string: i don't like the string in text format because (1) it is not concise (only the first item has the word 'Blocks'), (2) what block is it about? Shared, Local, or Temp?, (3) why don't you include the other ones?, and (4) why don't you include the written counters? -> Seq Scan on pg_namespace nc (cost=0.00..1.07 rows=4 width=68) (actual time=0.015..0.165 rows=4 loops=1) Filter: (NOT pg_is_other_temp_schema(oid)) Blocks Hit: 11 Read: 0 Temp Read: 0 (iv) text format: i don't have a good suggestion but here are some ideas. The former is too long and the latter is too verbose. :( Another option is to suppress words hit, read, and written; and just document it. Shared Blocks (11 hit, 5 read, 0 written); Local Blocks (5 hit, 0 read, 0 written); Temp Blocks (0 read, 0 written) or Shared Blocks: 11 hit, 5 read, 0 written Local Blocks: 5 hit, 0 read, 0 written Temp Blocks: 0 read, 0 written (v) accumulative: i don't remember if we discussed it but is there a reason the number of buffers isn't accumulative? We already have cost and time that are both accumulative. I saw BufferUsageAccumDiff() function but didn't try to figure out why it isn't accumulating or passing the counters to parent nodes. euler=# explain (analyze true, buffers true) select * from pgbench_branches inner join pgbench_history using (bid) where bid > 100; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------ Hash Join (cost=1.02..18.62 rows=3 width=476) (actual time=0.136..0.136 rows=0 loops=1) Hash Cond: (pgbench_history.bid = pgbench_branches.bid) Blocks Hit: 2 Read: 0 Temp Read: 0 -> Seq Scan on pgbench_history (cost=0.00..15.50 rows=550 width=116) (actual time=0.034..0.034 rows=1 loops=1) Blocks Hit: 1 Read: 0 Temp Read: 0 -> Hash (cost=1.01..1.01 rows=1 width=364) (actual time=0.022..0.022 rows=0 loops=1) Blocks Hit: 0 Read: 0 Temp Read: 0 -> Seq Scan on pgbench_branches (cost=0.00..1.01 rows=1 width=364) (actual time=0.019..0.019 rows=0 loops=1) Filter: (bid > 100) Blocks Hit: 1 Read: 0 Temp Read: 0 Total runtime: 0.531 ms (11 rows) (vi) comment: the 'at start' is superfluous. Please, remove it. + long blks_hit; /* # of buffer hits at start */ + long blks_read; /* # of disk blocks read at start */ (vii) all nodes: I'm thinking if we need this information in all nodes (even in those nodes that don't read or write). It would be less verbose but it could complicate some parser's life. Of course, if we suppress this information, we need to include it on the top node even if we don't read or write in it. I didn't have time to adjust your patch per comments above but if you can address all of those issues I certainly could check your patch again. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers