Re: Hide 'Execution time' in EXPLAIN (COSTS OFF)

Hi,

In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
printed. Should we perhaps do the same for 'Execution time'? That'd make
it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
regression tests.

Currently the output for that is:
postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
QUERY PLAN
--------------------------------
Result (actual rows=1 loops=1)
Total runtime: 0.035 ms
(2 rows)

Leaving off the total runtime doesn't seem bad to me.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
> printed. Should we perhaps do the same for 'Execution time'? That'd make
> it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
> regression tests.

> Currently the output for that is:
> postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
> QUERY PLAN
> --------------------------------
> Result (actual rows=1 loops=1)
> Total runtime: 0.035 ms
> (2 rows)

> Leaving off the total runtime doesn't seem bad to me.

It seems a little weird to call it a "cost" ... but maybe that
ship has sailed given how we're treating the planning-time item.

I'm unconvinced that this'd add much to our regression testing capability,
though. The standard thing is to do an EXPLAIN to check the plan shape
and then run the query to see if it gets the right answer. Checking row
counts is pretty well subsumed by the latter, and is certainly not an
adequate substitute for it.

So on the whole, -1 ... this is an unintuitive and
non-backwards-compatible change that doesn't look like it buys much.

regards, tom lane

On 2014-06-03 15:08:15 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
> > printed. Should we perhaps do the same for 'Execution time'? That'd make
> > it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
> > regression tests.
>
> > Currently the output for that is:
> > postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
> > QUERY PLAN
> > --------------------------------
> > Result (actual rows=1 loops=1)
> > Total runtime: 0.035 ms
> > (2 rows)
>
> > Leaving off the total runtime doesn't seem bad to me.
>
> It seems a little weird to call it a "cost" ... but maybe that
> ship has sailed given how we're treating the planning-time item.

It's not what I'd have choosen when starting afresh, but as you say...

> I'm unconvinced that this'd add much to our regression testing capability,
> though. The standard thing is to do an EXPLAIN to check the plan shape
> and then run the query to see if it gets the right answer. Checking row
> counts is pretty well subsumed by the latter, and is certainly not an
> adequate substitute for it.

The specific case I wanted it for was to test that a CREATE INDEX in a
specific situation actually has indexed a recently dead row. That can be
made visible via bitmap index scans... Generally index vs heap cases
aren't that easy to check with just the toplevel result.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
>> printed. Should we perhaps do the same for 'Execution time'? That'd make
>> it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
>> regression tests.
>
>> Currently the output for that is:
>> postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
>> QUERY PLAN
>> --------------------------------
>> Result (actual rows=1 loops=1)
>> Total runtime: 0.035 ms
>> (2 rows)
>
>> Leaving off the total runtime doesn't seem bad to me.
>
> It seems a little weird to call it a "cost" ... but maybe that
> ship has sailed given how we're treating the planning-time item.

Maybe we could make it be controlled by TIMING. Seems like it fits
well-enough there.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> It seems a little weird to call it a "cost" ... but maybe that
>> ship has sailed given how we're treating the planning-time item.

> Maybe we could make it be controlled by TIMING. Seems like it fits
> well-enough there.

Yeah, I thought about that too; but that sacrifices capability in the name
of terminological consistency. The point of TIMING OFF is to not pay the
very high overhead of per-node timing calls ... but that doesn't mean you
don't want the overall runtime. And it might not be convenient to get it
via client-side measurement.

regards, tom lane

On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>>> In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
>>> printed. Should we perhaps do the same for 'Execution time'? That'd
>make
>>> it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
>>> regression tests.
>>
>>> Currently the output for that is:
>>> postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
>>> QUERY PLAN
>>> --------------------------------
>>> Result (actual rows=1 loops=1)
>>> Total runtime: 0.035 ms
>>> (2 rows)
>>
>>> Leaving off the total runtime doesn't seem bad to me.
>>
>> It seems a little weird to call it a "cost" ... but maybe that
>> ship has sailed given how we're treating the planning-time item.
>
>Maybe we could make it be controlled by TIMING. Seems like it fits
>well-enough there.

Don't think that fits well - TIMING imo is about reducing the timing overhead. But the server side total time is still interesting. I only thought about tacking it onto COST because that already is pretty much tailored for regression test usage. C.F. disabling the planning time.

Andres

--
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund wrote:
> On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> >Maybe we could make it be controlled by TIMING. Seems like it fits
> >well-enough there.
>
> Don't think that fits well - TIMING imo is about reducing the timing
> overhead. But the server side total time is still interesting. I only
> thought about tacking it onto COST because that already is pretty much
> tailored for regression test usage. C.F. disabling the planning time.

Pah. So what we need is a new mode, REGRESSTEST ON or something.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jun 3, 2014 at 4:02 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> Andres Freund wrote:
>> On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> >Maybe we could make it be controlled by TIMING. Seems like it fits
>> >well-enough there.
>>
>> Don't think that fits well - TIMING imo is about reducing the timing
>> overhead. But the server side total time is still interesting. I only
>> thought about tacking it onto COST because that already is pretty much
>> tailored for regression test usage. C.F. disabling the planning time.
>
> Pah. So what we need is a new mode, REGRESSTEST ON or something.

Well, we could invent that. But I personally think piggybacking on
COSTS makes more sense.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Jun 3, 2014 at 4:02 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>> Pah. So what we need is a new mode, REGRESSTEST ON or something.

> Well, we could invent that. But I personally think piggybacking on
> COSTS makes more sense.

I've been eagerly waiting for 8.4 to die so I could stop worrying
about how far back I can back-patch regression test cases using
"explain (costs off)". It'd be really annoying to have to wait
another five years to get a consistent new spelling of how to do
that. So yeah, let's stick to using COSTS OFF in the tests.

regards, tom lane

On Tue, Jun 03, 2014 at 08:05:48PM +0200, Andres Freund wrote:
> In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
> printed. Should we perhaps do the same for 'Execution time'? That'd make
> it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
> regression tests.

I have wanted and would use such an option. I don't have a definite opinion
about how to spell the option.

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

On 2014-06-03 15:08:15 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > In 9.4. COSTS OFF for EXPLAIN prevents 'Planning time' to be
> > printed. Should we perhaps do the same for 'Execution time'? That'd make
> > it possible to use EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) in
> > regression tests.
>
> > Currently the output for that is:
> > postgres=# EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) SELECT 1;
> > QUERY PLAN
> > --------------------------------
> > Result (actual rows=1 loops=1)
> > Total runtime: 0.035 ms
> > (2 rows)
>
> > Leaving off the total runtime doesn't seem bad to me.
>
> It seems a little weird to call it a "cost" ... but maybe that
> ship has sailed given how we're treating the planning-time item.
>
> I'm unconvinced that this'd add much to our regression testing capability,
> though. The standard thing is to do an EXPLAIN to check the plan shape
> and then run the query to see if it gets the right answer. Checking row
> counts is pretty well subsumed by the latter, and is certainly not an
> adequate substitute for it.
>
> So on the whole, -1 ... this is an unintuitive and
> non-backwards-compatible change that doesn't look like it buys much.

I've added the regression test I want this for.

0001 is the bugfix making me look into it
0002 is COSTS OFF removing the display of execution time
0003 is the regression test

Note that 0003 will require a kill -9 without 0001.

I am not sure myself if the test is really worth it. On one hand it's an
area that had seen several hard to find bugs over the years and is
likely to see further changes (e.g. CSN stuff) in the near future, on
the other hand the tests are tricky and require specific ordering.

Opinions?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Re: Andres Freund 2014-06-04 <20140604194544(dot)GB785(at)awork2(dot)anarazel(dot)de>
> > I'm unconvinced that this'd add much to our regression testing capability,
> > though. The standard thing is to do an EXPLAIN to check the plan shape
> > and then run the query to see if it gets the right answer. Checking row
> > counts is pretty well subsumed by the latter, and is certainly not an
> > adequate substitute for it.
> >
> > So on the whole, -1 ... this is an unintuitive and
> > non-backwards-compatible change that doesn't look like it buys much.
>
> I've added the regression test I want this for.
>
> 0001 is the bugfix making me look into it
> 0002 is COSTS OFF removing the display of execution time
> 0003 is the regression test
>
> Note that 0003 will require a kill -9 without 0001.
>
> I am not sure myself if the test is really worth it. On one hand it's an
> area that had seen several hard to find bugs over the years and is
> likely to see further changes (e.g. CSN stuff) in the near future, on
> the other hand the tests are tricky and require specific ordering.

Hi,

there's another problem in this area: 9.4 adds "Planning time" to the
EXPLAIN output. If you don't want to see that, you need to use (costs
off), but this, well, also disables the costs. If you are running
regression tests to actually test the costs, you've lost in 9.4.

This problem just emerged in the Multicorn FDW where the regression
tests were monitoring the costs, but in 9.4 (costs off) kills that.

https://github.com/Kozea/Multicorn/pull/7

Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
line? That would even be backwards compatible with 9.x where it would
be a no-op.

(I don't have an opinion how much this should affect the "EXPLAIN
(analyze, timing off)" output, but there's probably a sane solution.)

Christoph
--
cb(at)df7cb(dot)de | http://www.df7cb.de/

On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> there's another problem in this area: 9.4 adds "Planning time" to the
> EXPLAIN output. If you don't want to see that, you need to use (costs
> off), but this, well, also disables the costs. If you are running
> regression tests to actually test the costs, you've lost in 9.4.
>
> This problem just emerged in the Multicorn FDW where the regression
> tests were monitoring the costs, but in 9.4 (costs off) kills that.
>
> https://github.com/Kozea/Multicorn/pull/7
>
> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
> line? That would even be backwards compatible with 9.x where it would
> be a no-op.

I don't think that'll work becuase:

/* check that timing is used with EXPLAIN ANALYZE */
if (es.timing && !es.analyze)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("EXPLAIN option TIMING
requires ANALYZE")));

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
>> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
>> line? That would even be backwards compatible with 9.x where it would
>> be a no-op.

> I don't think that'll work becuase:

> /* check that timing is used with EXPLAIN ANALYZE */
> if (es.timing && !es.analyze)
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("EXPLAIN option TIMING
> requires ANALYZE")));

It looks to me like that would complain about EXPLAIN (TIMING ON),
not the case Christoph is suggesting. What he proposes seems a bit
odd and non-orthogonal, but we could make the code do it if we wanted.

regards, tom lane

On Tue, Sep 23, 2014 at 1:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
>>> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
>>> line? That would even be backwards compatible with 9.x where it would
>>> be a no-op.
>
>> I don't think that'll work becuase:
>
>> /* check that timing is used with EXPLAIN ANALYZE */
>> if (es.timing && !es.analyze)
>> ereport(ERROR,
>> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> errmsg("EXPLAIN option TIMING
>> requires ANALYZE")));
>
> It looks to me like that would complain about EXPLAIN (TIMING ON),
> not the case Christoph is suggesting. What he proposes seems a bit
> odd and non-orthogonal, but we could make the code do it if we wanted.

Ah, right.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Tom Lane 2014-09-23 <15155(dot)1411493559(at)sss(dot)pgh(dot)pa(dot)us>
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> >> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
> >> line? That would even be backwards compatible with 9.x where it would
> >> be a no-op.
>
> > I don't think that'll work becuase:
>
> > /* check that timing is used with EXPLAIN ANALYZE */
> > if (es.timing && !es.analyze)
> > ereport(ERROR,
> > (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > errmsg("EXPLAIN option TIMING
> > requires ANALYZE")));
>
> It looks to me like that would complain about EXPLAIN (TIMING ON),
> not the case Christoph is suggesting. What he proposes seems a bit
> odd and non-orthogonal, but we could make the code do it if we wanted.

I don't think this warrants a new flag, and TIMING OFF seems to be the
right naming for it. (In fact it was the first I tried, and I was
cursing quite a bit over the lack of configurability until I realized
that COSTS OFF disabled the planning time display as well.) It might
be a bit odd, but it's easy to remember.

Christoph
--
cb(at)df7cb(dot)de | http://www.df7cb.de/

On Wed, Sep 24, 2014 at 8:02 AM, Christoph Berg <cb(at)df7cb(dot)de> wrote:

> Re: Tom Lane 2014-09-23 <15155(dot)1411493559(at)sss(dot)pgh(dot)pa(dot)us>
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > > On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> > >> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
> > >> line? That would even be backwards compatible with 9.x where it would
> > >> be a no-op.
> >
> > > I don't think that'll work becuase:
> >
> > > /* check that timing is used with EXPLAIN ANALYZE */
> > > if (es.timing && !es.analyze)
> > > ereport(ERROR,
> > >
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > > errmsg("EXPLAIN option TIMING
> > > requires ANALYZE")));
> >
> > It looks to me like that would complain about EXPLAIN (TIMING ON),
> > not the case Christoph is suggesting. What he proposes seems a bit
> > odd and non-orthogonal, but we could make the code do it if we wanted.
>
> I don't think this warrants a new flag, and TIMING OFF seems to be the
> right naming for it. (In fact it was the first I tried, and I was
> cursing quite a bit over the lack of configurability until I realized
> that COSTS OFF disabled the planning time display as well.) It might
> be a bit odd, but it's easy to remember.
>
>

I'm pretty interested in seeing something change around here.
The patch I'm working on at the moment (INNER JOIN removals) implements
"skipping" of joins at execution time rather than planning time. Currently
I'm working on the regression test for this and it's not all that easy due
to the execution time appearing in the results.

An explain analyze output from master with the patch can look something
like:

explain (analyze, costs off, timing off)
select a.* from a inner join b on a.b_id = b.id inner join c on b.c_id =
c.id;
QUERY PLAN
---------------------------------------------------
Hash Join (actual rows=1 loops=1)
Hash Cond: (b.c_id = c.id)
-> Hash Join (actual rows=1 loops=1)
Hash Cond: (a.b_id = b.id)
-> Seq Scan on a (actual rows=1 loops=1)
-> Hash (never executed)
-> Seq Scan on b (never executed)
-> Hash (never executed)
-> Seq Scan on c (never executed)
Execution time: 0.092 ms
(10 rows)

From this I can see easily that the joins to b and c were skipped, however
the output the way it is at the moment is quite useless for regression
testing with.

Regards

David Rowley

On 2014-10-12 23:13:27 +1300, David Rowley wrote:
> On Wed, Sep 24, 2014 at 8:02 AM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
>
> > Re: Tom Lane 2014-09-23 <15155(dot)1411493559(at)sss(dot)pgh(dot)pa(dot)us>
> > > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > > > On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> > > >> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
> > > >> line? That would even be backwards compatible with 9.x where it would
> > > >> be a no-op.
> > >
> > > > I don't think that'll work becuase:
> > >
> > > > /* check that timing is used with EXPLAIN ANALYZE */
> > > > if (es.timing && !es.analyze)
> > > > ereport(ERROR,
> > > >
> > (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > > > errmsg("EXPLAIN option TIMING
> > > > requires ANALYZE")));
> > >
> > > It looks to me like that would complain about EXPLAIN (TIMING ON),
> > > not the case Christoph is suggesting. What he proposes seems a bit
> > > odd and non-orthogonal, but we could make the code do it if we wanted.
> >
> > I don't think this warrants a new flag, and TIMING OFF seems to be the
> > right naming for it. (In fact it was the first I tried, and I was
> > cursing quite a bit over the lack of configurability until I realized
> > that COSTS OFF disabled the planning time display as well.) It might
> > be a bit odd, but it's easy to remember.
> >
> >
>
> I'm pretty interested in seeing something change around here.
> The patch I'm working on at the moment (INNER JOIN removals) implements
> "skipping" of joins at execution time rather than planning time. Currently
> I'm working on the regression test for this and it's not all that easy due
> to the execution time appearing in the results.
>
> An explain analyze output from master with the patch can look something
> like:
>
> explain (analyze, costs off, timing off)
> select a.* from a inner join b on a.b_id = b.id inner join c on b.c_id =
> c.id;
> QUERY PLAN
> ---------------------------------------------------
> Hash Join (actual rows=1 loops=1)
> Hash Cond: (b.c_id = c.id)
> -> Hash Join (actual rows=1 loops=1)
> Hash Cond: (a.b_id = b.id)
> -> Seq Scan on a (actual rows=1 loops=1)
> -> Hash (never executed)
> -> Seq Scan on b (never executed)
> -> Hash (never executed)
> -> Seq Scan on c (never executed)
> Execution time: 0.092 ms
> (10 rows)

So you're now the third person reporting problems here. Let's remove
'execution time' for COSTS off.

I personally would even say that we should backpatch that to make
backpatches involving regression tests less painful.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Le dimanche 12 octobre 2014 13:17:00 Andres Freund a écrit :
> On 2014-10-12 23:13:27 +1300, David Rowley wrote:
> > On Wed, Sep 24, 2014 at 8:02 AM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> > > Re: Tom Lane 2014-09-23 <15155(dot)1411493559(at)sss(dot)pgh(dot)pa(dot)us>
> > >
> > > > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > > > > On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb(at)df7cb(dot)de> wrote:
> > > > >> Can we have "EXPLAIN (timing off)" in 9.4+ hide the "Planning time"
> > > > >> line? That would even be backwards compatible with 9.x where it
> > > > >> would
> > > > >> be a no-op.
> > > > >
> > > > > I don't think that'll work becuase:
> > > > > /* check that timing is used with EXPLAIN ANALYZE */
> > > > > if (es.timing && !es.analyze)
> > > > >
> > > > > ereport(ERROR,
> > >
> > > (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > >
> > > > > errmsg("EXPLAIN option TIMING
> > > > >
> > > > > requires ANALYZE")));
> > > >
> > > > It looks to me like that would complain about EXPLAIN (TIMING ON),
> > > > not the case Christoph is suggesting. What he proposes seems a bit
> > > > odd and non-orthogonal, but we could make the code do it if we wanted.
> > >
> > > I don't think this warrants a new flag, and TIMING OFF seems to be the
> > > right naming for it. (In fact it was the first I tried, and I was
> > > cursing quite a bit over the lack of configurability until I realized
> > > that COSTS OFF disabled the planning time display as well.) It might
> > > be a bit odd, but it's easy to remember.
> >
> > I'm pretty interested in seeing something change around here.
> > The patch I'm working on at the moment (INNER JOIN removals) implements
> > "skipping" of joins at execution time rather than planning time. Currently
> > I'm working on the regression test for this and it's not all that easy due
> > to the execution time appearing in the results.
> >
> > An explain analyze output from master with the patch can look something
> > like:
> >
> > explain (analyze, costs off, timing off)
> > select a.* from a inner join b on a.b_id = b.id inner join c on b.c_id =
> > c.id;
> >
> > QUERY PLAN
> >
> > ---------------------------------------------------
> >
> > Hash Join (actual rows=1 loops=1)
> >
> > Hash Cond: (b.c_id = c.id)
> > -> Hash Join (actual rows=1 loops=1)
> >
> > Hash Cond: (a.b_id = b.id)
> > -> Seq Scan on a (actual rows=1 loops=1)
> > -> Hash (never executed)
> >
> > -> Seq Scan on b (never executed)
> >
> > -> Hash (never executed)
> >
> > -> Seq Scan on c (never executed)
> >
> > Execution time: 0.092 ms
> >
> > (10 rows)
>
> So you're now the third person reporting problems here. Let's remove
> 'execution time' for COSTS off.
>
> I personally would even say that we should backpatch that to make
> backpatches involving regression tests less painful.

That wouldn't solve the first problem mentioned, which is that for some
regression tests one may want to test the costs themselves, which is now
impossible with the new planning time feature.

What would IMO make both cases suitable would be to eliminate ALL timing from
TIMING OFF, not only the timing on the individual nodes. As was mentioned
before, it is a bit counter intuitive to have COSTS OFF disable the planning
time, and not TIMING OFF.

>
> Greetings,
>
> Andres Freund

--
Ronan Dunklau

Ronan Dunklau <ronan(at)dunklau(dot)fr> writes:
> That wouldn't solve the first problem mentioned, which is that for some
> regression tests one may want to test the costs themselves, which is now
> impossible with the new planning time feature.

That's a bogus argument, because it was impossible before too. We have
no such tests now, and it's unlikely we will ever add any, because costs
inherently are platform-dependent. The reason we invented COSTS OFF in
the first place was to make it possible to do EXPLAIN in regression tests
without getting platform-dependent output.

I have no great objection to making both COSTS OFF and TIMING OFF suppress
the "planning time" output, if that's the consensus. I would object to
taking away that behavior of COSTS OFF, because of the implications for
back-patching EXPLAIN queries in regression tests.

Another possibility, which would introduce less non-orthogonality into
the switch design, is to remove the connection to COSTS OFF but say that
planning time is only printed when execution time is also printed (ie,
only in EXPLAIN ANALYZE). This seems to me that it would not be removing
much functionality, because if you just did a plain EXPLAIN then you can
take the client-side runtime (psql \timing) as a close-enough estimate
of planning time.

regards, tom lane

On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Ronan Dunklau <ronan(at)dunklau(dot)fr> writes:
>> That wouldn't solve the first problem mentioned, which is that for some
>> regression tests one may want to test the costs themselves, which is now
>> impossible with the new planning time feature.
>
> That's a bogus argument, because it was impossible before too. We have
> no such tests now, and it's unlikely we will ever add any, because costs
> inherently are platform-dependent. The reason we invented COSTS OFF in
> the first place was to make it possible to do EXPLAIN in regression tests
> without getting platform-dependent output.
>
> I have no great objection to making both COSTS OFF and TIMING OFF suppress
> the "planning time" output, if that's the consensus. I would object to
> taking away that behavior of COSTS OFF, because of the implications for
> back-patching EXPLAIN queries in regression tests.
>
> Another possibility, which would introduce less non-orthogonality into
> the switch design, is to remove the connection to COSTS OFF but say that
> planning time is only printed when execution time is also printed (ie,
> only in EXPLAIN ANALYZE). This seems to me that it would not be removing
> much functionality, because if you just did a plain EXPLAIN then you can
> take the client-side runtime (psql \timing) as a close-enough estimate
> of planning time.

That'd be fine with me. Making it controlled by COSTS and/or TIMING
would be OK with me, too. But let's do *something*.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: Tom Lane 2014-10-12 <19766(dot)1413129321(at)sss(dot)pgh(dot)pa(dot)us>
> Another possibility, which would introduce less non-orthogonality into
> the switch design, is to remove the connection to COSTS OFF but say that
> planning time is only printed when execution time is also printed (ie,
> only in EXPLAIN ANALYZE). This seems to me that it would not be removing
> much functionality, because if you just did a plain EXPLAIN then you can
> take the client-side runtime (psql \timing) as a close-enough estimate
> of planning time.

I like that idea. Also, while the planning time is real even when
doing a plain EXPLAIN, people who are interested in runtime behavior
will be running full EXPLAIN (ANALYZE) anyway.

My original suggestion to let (TIMING OFF) control it would allow for
more flexibility, but as noted it isn't 100% in line with the other
options, and this "new" idea should even be much simpler to implement
or maintain.

Christoph
--
cb(at)df7cb(dot)de | http://www.df7cb.de/

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I have no great objection to making both COSTS OFF and TIMING OFF suppress
>> the "planning time" output, if that's the consensus. I would object to
>> taking away that behavior of COSTS OFF, because of the implications for
>> back-patching EXPLAIN queries in regression tests.
>>
>> Another possibility, which would introduce less non-orthogonality into
>> the switch design, is to remove the connection to COSTS OFF but say that
>> planning time is only printed when execution time is also printed (ie,
>> only in EXPLAIN ANALYZE). This seems to me that it would not be removing
>> much functionality, because if you just did a plain EXPLAIN then you can
>> take the client-side runtime (psql \timing) as a close-enough estimate
>> of planning time.

> That'd be fine with me. Making it controlled by COSTS and/or TIMING
> would be OK with me, too. But let's do *something*.

After sleeping on it, the second idea seems cleaner to me: it removes one
wart rather than adding a second one. If there are no objections, I'll
go make it so.

regards, tom lane

On 2014-10-13 11:46:16 -0400, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I have no great objection to making both COSTS OFF and TIMING OFF suppress
> >> the "planning time" output, if that's the consensus. I would object to
> >> taking away that behavior of COSTS OFF, because of the implications for
> >> back-patching EXPLAIN queries in regression tests.
> >>
> >> Another possibility, which would introduce less non-orthogonality into
> >> the switch design, is to remove the connection to COSTS OFF but say that
> >> planning time is only printed when execution time is also printed (ie,
> >> only in EXPLAIN ANALYZE). This seems to me that it would not be removing
> >> much functionality, because if you just did a plain EXPLAIN then you can
> >> take the client-side runtime (psql \timing) as a close-enough estimate
> >> of planning time.
>
> > That'd be fine with me. Making it controlled by COSTS and/or TIMING
> > would be OK with me, too. But let's do *something*.
>
> After sleeping on it, the second idea seems cleaner to me: it removes one
> wart rather than adding a second one. If there are no objections, I'll
> go make it so.

Well. Unless I miss something it doesn't resolve the problem that
started this thread. Namely that it's currently impossible to write
regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is
worthwhile because it allows to tests some behaviour that's only visible
in actually executed plans (like seing that a subtree wasn't executed).

I think we should try to find a solution that solves the problem for
execution/plan time problem at the same time...

We could just make TIMING a tristate variable (really-off, off, on). Not
very satisfying given that we have to preserve the current behaviour
with OFF :(.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> Well. Unless I miss something it doesn't resolve the problem that
> started this thread. Namely that it's currently impossible to write
> regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is
> worthwhile because it allows to tests some behaviour that's only visible
> in actually executed plans (like seing that a subtree wasn't executed).

TBH, I don't particularly care about that. A new flag for turning
"summary timing" off would answer the complaint with not too much
non-orthogonality ... but I really don't find this use case compelling
enough to justify adding a feature to EXPLAIN.

regards, tom lane

On Tue, Oct 14, 2014 at 6:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > Well. Unless I miss something it doesn't resolve the problem that
> > started this thread. Namely that it's currently impossible to write
> > regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is
> > worthwhile because it allows to tests some behaviour that's only visible
> > in actually executed plans (like seing that a subtree wasn't executed).
>
> TBH, I don't particularly care about that. A new flag for turning
> "summary timing" off would answer the complaint with not too much
> non-orthogonality ... but I really don't find this use case compelling
> enough to justify adding a feature to EXPLAIN.
>
>
Hmm, was my case above not compelling enough?
This leaves me out in the cold a bit for when it comes to testing inner
joins are properly skipped at execution time. I can see no other way to
properly verify when the joins are and are not being skipped other than
outputting the explain analyze in the test and I can't really imagine it
ever getting committed without proper regression tests.

Can you think of some other way that I could test this? Keep in mind
there's no trace of the removal in the EXPLAIN without ANALYZE.

Regards

David Rowley

On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> > Well. Unless I miss something it doesn't resolve the problem that
>> > started this thread. Namely that it's currently impossible to write
>> > regression using EXPLAIN (ANALYZE, TIMING OFF. COSTS OFF). Which is
>> > worthwhile because it allows to tests some behaviour that's only visible
>> > in actually executed plans (like seing that a subtree wasn't executed).
>>
>> TBH, I don't particularly care about that. A new flag for turning
>> "summary timing" off would answer the complaint with not too much
>> non-orthogonality ... but I really don't find this use case compelling
>> enough to justify adding a feature to EXPLAIN.
>>
> Hmm, was my case above not compelling enough?

Apparently not to Tom, but it made sense to me. I think we should
find a way to do something about this - maybe making TIMING OFF also
suppress that info is the simplest approach.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>> Hmm, was my case above not compelling enough?

> Apparently not to Tom, but it made sense to me.

No, it wasn't. I'm not convinced either that that patch will get in at
all, or that it has to have regression tests of that particular form,
or that such a switch would be sufficient to make such tests platform
independent.

> I think we should
> find a way to do something about this - maybe making TIMING OFF also
> suppress that info is the simplest approach.

We intentionally did *not* make TIMING OFF do that to begin with, and
I think changing that behavior now has even less chance of escaping
push-back than the "planning time" change did.

If we're convinced we must do something, I think exposing the SUMMARY
ON/OFF flag (possibly after bikeshedding the name) that I implemented
internally yesterday would be the thing to do. But as I said, I find
the use-case for this pretty darn weak.

regards, tom lane

On 2014-10-16 10:06:59 -0400, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> >> Hmm, was my case above not compelling enough?
>
> > Apparently not to Tom, but it made sense to me.
>
> No, it wasn't. I'm not convinced either that that patch will get in at
> all, or that it has to have regression tests of that particular form,
> or that such a switch would be sufficient to make such tests platform
> independent.

It's not like we don't already have features where that capability
actually would be useful. E.g. testing that certain nodes aren't reached
during execution and instead '(never executed)' and things like that.

Why should the EXPLAIN ANALYZE output without timing information be less
consistent for sensibly selected cases than EXPLAIN itself? I'd actually
say stats are harder to get right than the actual number of rows.

There also have been bugs where this capability would have been
useful. And don't say that regression testing wouldn't have helped there
- the case I'm thinking of was broken several times over the years.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-10-16 10:06:59 -0400, Tom Lane wrote:
>> No, it wasn't. I'm not convinced either that that patch will get in at
>> all, or that it has to have regression tests of that particular form,
>> or that such a switch would be sufficient to make such tests platform
>> independent.

> Why should the EXPLAIN ANALYZE output without timing information be less
> consistent for sensibly selected cases than EXPLAIN itself?

To take just one example, the performance numbers that get printed for
a sort, such as memory consumption, are undoubtedly platform-dependent.
Maybe your idea of "sensibly selected cases" excludes sorting, but
I don't find such an argument terribly convincing. I think if we go
down this road, we are going to end up with an EXPLAIN that has one
hundred parameters turning on and off tiny pieces of the output, none
of which are of any great use for anything except the regression tests.
I don't want to go there. It would be a lot better to expend the effort
on a better regression testing infrastructure that wouldn't *need*
bitwise-identical output across platforms. (mysql is ahead of us in that
department: they have some hacks for selective matching of the output.)

regards, tom lane

On Thu, Oct 16, 2014 at 10:06 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>>> Hmm, was my case above not compelling enough?
>
>> Apparently not to Tom, but it made sense to me.
>
> No, it wasn't. I'm not convinced either that that patch will get in at
> all, or that it has to have regression tests of that particular form,
> or that such a switch would be sufficient to make such tests platform
> independent.

People clearly want to be able to run EXPLAIN (ANALYZE) and get stable
output. If the proposed change isn't enough to make that happen, we
need to do more, not give up. Regardless of what happens to inner
join removal.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Le jeudi 16 octobre 2014 10:43:25 Robert Haas a écrit :
> On Thu, Oct 16, 2014 at 10:06 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >> On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml(at)gmail(dot)com>
wrote:
> >>> Hmm, was my case above not compelling enough?
> >>
> >> Apparently not to Tom, but it made sense to me.
> >
> > No, it wasn't. I'm not convinced either that that patch will get in at
> > all, or that it has to have regression tests of that particular form,
> > or that such a switch would be sufficient to make such tests platform
> > independent.
>
> People clearly want to be able to run EXPLAIN (ANALYZE) and get stable
> output. If the proposed change isn't enough to make that happen, we
> need to do more, not give up. Regardless of what happens to inner
> join removal.

From my point of view as a FDW implementor, the feature I need is to have
EXPLAIN (COSTS ON) with stable output for foreign scan nodes.

In the Multicorn FDW (Python API on top of the C-API), we introduced this
commit to make the tests pass on 9.4:

https://github.com/Kozea/Multicorn/commit/76decb360b822b57bf322892ed6c504ba44a8b28

Clearly, we've lost the ability to test that the costs as set from the Python
API are indeed used.

But I agree that it would be better to have more flexibility in the regression
framework itself.

If this use case is too marginal to warrant such a change, I'll keep the tests
as they are now.

--
Ronan Dunklau
http://dalibo.com - http://dalibo.org

Ronan Dunklau <ronan(dot)dunklau(at)dalibo(dot)com> writes:
> From my point of view as a FDW implementor, the feature I need is to have
> EXPLAIN (COSTS ON) with stable output for foreign scan nodes.

Well, as long as the FDW's costing is exactly predictable, you can have
that ...

> In the Multicorn FDW (Python API on top of the C-API), we introduced this
> commit to make the tests pass on 9.4:

> https://github.com/Kozea/Multicorn/commit/76decb360b822b57bf322892ed6c504ba44a8b28

> Clearly, we've lost the ability to test that the costs as set from the Python
> API are indeed used.

We did fix that yesterday. The remaining argument is about whether it's
practical to get platform-independent output out of EXPLAIN ANALYZE.

regards, tom lane

On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-10-16 10:06:59 -0400, Tom Lane wrote:
> >> No, it wasn't. I'm not convinced either that that patch will get in at
> >> all, or that it has to have regression tests of that particular form,
> >> or that such a switch would be sufficient to make such tests platform
> >> independent.
>
> > Why should the EXPLAIN ANALYZE output without timing information be less
> > consistent for sensibly selected cases than EXPLAIN itself?
>
> To take just one example, the performance numbers that get printed for
> a sort, such as memory consumption, are undoubtedly platform-dependent.
> Maybe your idea of "sensibly selected cases" excludes sorting, but
> I don't find such an argument terribly convincing. I think if we go
> down this road, we are going to end up with an EXPLAIN that has one
> hundred parameters turning on and off tiny pieces of the output, none
> of which are of any great use for anything except the regression tests.
> I don't want to go there. It would be a lot better to expend the effort
> on a better regression testing infrastructure that wouldn't *need*
> bitwise-identical output across platforms. (mysql is ahead of us in that
> department: they have some hacks for selective matching of the output.)
>
>
I saw this, and I was about to ask the same question as Andres.... I think
I now see what you're worried about. Next we'd need a flag to disable
external disk sort sizes too...

Perhaps we could introduce some sort of wildcard matching in the regression
tests. So that we could stick something like:

Execution time: * ms

Into the expected results, though, probably we'd need to come up with some
wildcard character which is a bit less common than *

It might be hard to generate a useful diff with this for when a test fails,
but maybe it'd be good enough to just run the 2 files through this wildcard
matching programme and then just do a diff if it fails.

Regards

David Rowley

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I don't want to go there. It would be a lot better to expend the effort
>> on a better regression testing infrastructure that wouldn't *need*
>> bitwise-identical output across platforms. (mysql is ahead of us in that
>> department: they have some hacks for selective matching of the output.)

> Perhaps we could introduce some sort of wildcard matching in the regression
> tests. So that we could stick something like:
> Execution time: * ms
> Into the expected results, though, probably we'd need to come up with some
> wildcard character which is a bit less common than *

I was imagining that we might as well allow regexp matching, so you could
be as specific as

Execution time: \d+\.\d+ ms

if you had a mind to. But with or without that, it would be difficult to
pick a meta-character that never appears in expected-output files today.
What we'd probably want to do (again, I'm stealing ideas from what I
remember of the mysql regression tests) is add metasyntax to switch
between literal and wildcard/regexp matching. So perhaps an expected
file could contain something like

-- !!match regexp
... expected output including regexps ...
-- !!match literal
... normal expected output here

Not sure how we get there without writing our own diff engine though :-(.

regards, tom lane

On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I don't want to go there. It would be a lot better to expend the effort
> >> on a better regression testing infrastructure that wouldn't *need*
> >> bitwise-identical output across platforms. (mysql is ahead of us in
> that
> >> department: they have some hacks for selective matching of the output.)
>
> > Perhaps we could introduce some sort of wildcard matching in the
> regression
> > tests. So that we could stick something like:
> > Execution time: * ms
> > Into the expected results, though, probably we'd need to come up with
> some
> > wildcard character which is a bit less common than *
>
> I was imagining that we might as well allow regexp matching, so you could
> be as specific as
>
> Execution time: \d+\.\d+ ms
>
> if you had a mind to. But with or without that, it would be difficult to
> pick a meta-character that never appears in expected-output files today.
> What we'd probably want to do (again, I'm stealing ideas from what I
> remember of the mysql regression tests) is add metasyntax to switch
> between literal and wildcard/regexp matching. So perhaps an expected
> file could contain something like
>
> -- !!match regexp
> ... expected output including regexps ...
> -- !!match literal
> ... normal expected output here
>
>
That seems better, that way we'd be safer from accidentally matching when
we shouldn't

> Not sure how we get there without writing our own diff engine though :-(.
>
>
I had imagined that we wouldn't need this, but perhaps my workflow is just
different from yours. When I make changes which make tests fail for a valid
reason I'd use beyondcompare to cherrypick the actual back into the
expected, but I suppose others might just apply the diff into the
expected.... Umm, but then wouldn't you just copy the whole actual file
over to expected?... So why do we need diffs? Couldn't this matching tool
just report where the first non-matching line appeared in the file? We
could then manually diff the files and just ignore the !!match stuff and
the regex differences.

Would that kill anyone's workflow?

Regards

David Rowley

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Not sure how we get there without writing our own diff engine though :-(.

(Note that after a bit of looking around, it seems like it might not be
that hard to do something like this in Perl. Perl is already
nearly-required for building from source, could we require it for running
the regression tests? Though we'd also need you to install
Algorithm::Diff or suchlike, which I think isn't in a basic Perl install.)

> I had imagined that we wouldn't need this, but perhaps my workflow is just
> different from yours. When I make changes which make tests fail for a valid
> reason I'd use beyondcompare to cherrypick the actual back into the
> expected, but I suppose others might just apply the diff into the
> expected.... Umm, but then wouldn't you just copy the whole actual file
> over to expected?

That's what I usually do, except when dealing with the ones that are
generated from output/ files. (Which are a PITA to update, and I guess
files containing wildcard matches would be too.)

> So why do we need diffs? Couldn't this matching tool
> just report where the first non-matching line appeared in the file?

Not too helpful for buildfarm reports. Nor for anyone else, really;
you're just pushing the problem of identifying the important difference
back onto the user.

regards, tom lane

On 2014-10-19 12:26:24 -0400, Tom Lane wrote:
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> Not sure how we get there without writing our own diff engine though :-(.
>
> (Note that after a bit of looking around, it seems like it might not be
> that hard to do something like this in Perl. Perl is already
> nearly-required for building from source, could we require it for running
> the regression tests? Though we'd also need you to install
> Algorithm::Diff or suchlike, which I think isn't in a basic Perl install.)

I personally don't mind that. but if we think it's a problem, we could
quite easily embed a copy of Algorithm::Diff - easy enough with perl.

If we feel the need, we could enable the feature optionally using
resultmap. That way regression tests wouldn't get detected optionally.

I still think that reducing the need for having to do this is a good
idea, having to manually edit regression output to add regexes will be a
PITA.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> I still think that reducing the need for having to do this is a good
> idea, having to manually edit regression output to add regexes will be a
> PITA.

Yeah :-(. We already have two features somewhat related to this, viz
prototype expected files in output/ and multiple expected files. And
each of them is a pain when it comes time to update the expected output.
So that analogy is damping my enthusiasm a bit. Still, it beats not
being able to make a test at all. And to the extent that we could get
rid of variant expected files, we could eliminate a class of committer
mistakes that's bitten pretty much everyone at one time or another...

regards, tom lane

On 2014-10-19 12:38:52 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > I still think that reducing the need for having to do this is a good
> > idea, having to manually edit regression output to add regexes will be a
> > PITA.
>
> Yeah :-(.

With regard to what triggered this subthread, I still think we should
remove more irreproducible output to avoid having to do that more
often...But since I also think regexes would really be useful for other
cases, that doesn't preclude going forward with regex diffs either.

> We already have two features somewhat related to this, viz
> prototype expected files in output/ and multiple expected files. And
> each of them is a pain when it comes time to update the expected output.
> So that analogy is damping my enthusiasm a bit. Still, it beats not
> being able to make a test at all. And to the extent that we could get
> rid of variant expected files, we could eliminate a class of committer
> mistakes that's bitten pretty much everyone at one time or another...

We probably can evn get rid of a fair bit of the output/ cases. The
input files still have to be generated, but it's still less work.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services