Florian Pflug <f...@phlo.org> writes: > On Jan22, 2011, at 17:55 , Tom Lane wrote: >> Reflecting on that, I'm inclined to suggest >> Bitmap Heap Scan ... >> Recheck Cond: blah blah >> Rows Removed by Recheck Cond: 42 >> Filter Cond: blah blah blah >> Rows Removed by Filter Cond: 77
> +1. Repeating the label of the condition adds enough context to make > "Removed" unambiguous IMHO. Given where we've ended up on what we want printed, I'm forced to conclude that there is basically nothing usable in the submitted patch. We have the following problems: 1. It only instruments the ExecQual() call in execScan.c. There are close to 20 other calls that also need instrumentation, unless we intentionally decide not to bother with counting results for certain filters (but see #4). 2. Putting the counter in struct Instrumentation doesn't scale very nicely to cases with more than one qual per node. We could put some arbitrary number of counters into that struct and make some arbitrary assignments of which is used for what, but ugh. I am tempted to suggest that these counters should go right into the PlanState nodes for the relevant plan node types; which would likely mean that they'd get incremented unconditionally whether we're running EXPLAIN ANALYZE or not. Offhand I don't have a problem with that --- it's not apparent to me that if (node->ps.instrument) node->counter += 1; is faster than an unconditional node->counter += 1; on modern machines in the first place, and in the second place we have certainly expended many more cycles than that by the time we have completed a failing ExecQual call, so it's unlikely to matter. 3. The additions to explain.c of course need complete revision to support this output style. Likewise the documentation (and as was mentioned upthread this isn't enough documentation anyway, since it utterly fails to explain what nfiltered is to users). 4. I'm not entirely sure what to do with nodeResult's resconstantqual. If we do instrument that, it would have to be done differently from everywhere else, since execQual is called only once not once per tuple. But on the whole I think we could leave it out, since it's pretty obvious from the node's overall behavior whether the qual passed or not. I had thought perhaps I could fix this patch up and commit it, but a complete rewrite seems beyond the bounds of what should happen during a CommitFest, especially since there are lots of other patches in need of attention. So I'm marking it Returned With Feedback. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers