Re: WIP: explain analyze with 'rows' but not timing

On 23.12.2011 14:57, Robert Haas wrote:
> 2011/12/22 Tomas Vondra <tv@fuzzy.cz>:
>> The gettimeofday() calls are not exactly cheap in some cases, so why to
>> pay that price when all you need is the number of rows?
>
> Fair point.
>
>> The patch attached does this:
>>
>> 1) adds INSTRUMENT_ROWS, a new InstrumentOption
>>
>>   - counts rows without timing (no gettimeofday() callse)
>>   - if you want timing info, use INSTRUMENT_TIMER
>>
>> 2) adds new option "TIMING" to EXPLAIN, i.e.
>>
>>    EXPLAIN (ANALYZE ON, TIMING ON) SELECT ...
>>
>> 3) adds auto_explain.log_rows_only (false by default)
>>
>>   - if you set this to 'true', then the instrumentation will just
>>     count rows, without calling gettimeofday()
>
> This seems like an unnecessarily confusing interface, because you've
> named the auto_explain option differently from the EXPLAIN option and
> given it (almost) the opposite sense: timing=off means the same thing
> as log_rows_only=on.
>
> I think the EXPLAIN (TIMING) option is good the way you have it, but
> then just have auto_explain.log_timing, with a default value of on.

Makes sense. I've updated the patch to reflect this, so the option is
called auto_explain.log_timing and is true by default. I'll add the
patch to the next commit fest.

One thing I'm wondering about is that the InstrumentOptions are not
exclusive - INSTRUMENT_TIMER means 'collect timing and row counts' while
INSTRUMENT_ROWS means 'collect row counts'. Wouldn't it be better to
redefine the INSTRUMENT_TIMER so that it collects just timing info. I.e.
to get the current behaviour, you'd have to do this

   instrument_options |= (INSTRUMENT_TIMER | INSTRUMENT_ROWS)

It's quite trivial change in explain.c, the problem I have with that is
that it might break extensions.

Tomas