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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-06-03 15:08:15 -0400, Tom Lane wrote:
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Jun 3, 2014 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@2ndquadrant.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'dmake
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund wrote:
On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jun 3, 2014 at 4:02 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Andres Freund wrote:
On June 3, 2014 9:40:27 PM CEST, Robert Haas <robertmhaas@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Jun 3, 2014 at 4:02 PM, Alvaro Herrera <alvherre@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-06-03 15:08:15 -0400, Tom Lane wrote:
Andres Freund <andres@2ndquadrant.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
Attachments:
0001-Fix-longstanding-bug-in-HeapTupleSatisfiesVacuum.patchtext/x-patch; charset=us-asciiDownload
>From 621a99a666ba1a27b852dc5ddc0e1b224c388f53 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 4 Jun 2014 21:36:19 +0200
Subject: [PATCH 1/3] Fix longstanding bug in HeapTupleSatisfiesVacuum().
HeapTupleSatisfiesVacuum() didn't properly discern between
DELETE_IN_PROGRESS and INSERT_IN_PROGRESS for rows that have been
inserted in the current transaction and deleted in a aborted
subtransaction of the current backend. At the very least that caused
problems for CLUSTER and CREATE INDEX in transactions that had
aborting subtransactions producing rows, leading to warnings like:
WARNING: concurrent delete in progress within table "..."
possibly in an endless, uninterruptible, loop.
Instead of treating *InProgress xmins the same as *IsCurrent ones,
treat them as being distinct like the other visibility routines. As
implemented this separatation can cause a behaviour change for rows
that have been inserted and deleted in another, still running,
transaction. HTSV will now return INSERT_IN_PROGRESS instead of
DELETE_IN_PROGRESS for those. That's both, more in line with the other
visibility routines and arguably more correct. The latter because a
INSERT_IN_PROGRESS will make callers look at/wait for xmin, instead of
xmax.
The only current caller where that's possibly worse than the old
behaviour is heap_prune_chain() which now won't mark the page as
prunable if a row has concurrently been inserted and deleted. That's
harmless enough.
As a cautionary measure also insert a interrupt check before the gotos
in IndexBuildHeapScan() that lead to the uninterruptible loop. There
are other possible causes, like a row that several sessions try to
update and all fail, for repeated loops and the cost of doing so in
the retry case is low.
As this bug goes back all the way to the introduction of
subtransactions in 573a71a5da backpatch to all supported releases.
Reported-By: Sandro Santilli
---
src/backend/catalog/index.c | 2 ++
src/backend/utils/time/tqual.c | 19 +++++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 80acc0e..a5a204e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2298,6 +2298,7 @@ IndexBuildHeapScan(Relation heapRelation,
XactLockTableWait(xwait, heapRelation,
&heapTuple->t_data->t_ctid,
XLTW_InsertIndexUnique);
+ CHECK_FOR_INTERRUPTS();
goto recheck;
}
}
@@ -2346,6 +2347,7 @@ IndexBuildHeapScan(Relation heapRelation,
XactLockTableWait(xwait, heapRelation,
&heapTuple->t_data->t_ctid,
XLTW_InsertIndexUnique);
+ CHECK_FOR_INTERRUPTS();
goto recheck;
}
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 75cd53e..96874ab 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -1166,7 +1166,7 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
return HEAPTUPLE_DEAD;
}
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
+ else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmin(tuple)))
{
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
return HEAPTUPLE_INSERT_IN_PROGRESS;
@@ -1175,7 +1175,22 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
HeapTupleHeaderIsOnlyLocked(tuple))
return HEAPTUPLE_INSERT_IN_PROGRESS;
/* inserted and then deleted by same xact */
- return HEAPTUPLE_DELETE_IN_PROGRESS;
+ if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetUpdateXid(tuple)))
+ return HEAPTUPLE_DELETE_IN_PROGRESS;
+ /* deleting subtransaction must have aborted */
+ return HEAPTUPLE_INSERT_IN_PROGRESS;
+ }
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
+ {
+ /*
+ * It'd be possible to discern between INSERT/DELETE in progress
+ * here by looking at xmax - but that doesn't seem beneficial for
+ * the majority of callers and even detrimental for some. We'd
+ * rather have callers look at/wait for xmin than xmax. It's
+ * always correct to return INSERT_IN_PROGRESS because that's
+ * what's happening from the view of other backends.
+ */
+ return HEAPTUPLE_INSERT_IN_PROGRESS;
}
else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
--
2.0.0.rc2.4.g1dc51c6.dirty
0002-Don-t-print-the-execution-time-for-EXPLAIN-ANALYZE-C.patchtext/x-patch; charset=us-asciiDownload
>From 093af92ea74dc3a8c3a3eb1f323b0c9508816a71 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 4 Jun 2014 21:36:19 +0200
Subject: [PATCH 2/3] Don't print the execution time for EXPLAIN (ANALYZE,
COSTS OFF).
The primary reason to use COSTS OFF for EXPLAIN is to get reproducable
output. Until now EXPLAIN with ANALYZE didn't generate reproducable
output, because it printed the execution time even with TIMING
OFF. Strangely sounding as it may be that actually makes sense because
TIMING OFF is there to reduce the timing overhead, not to produce
reproducable output.
---
src/backend/commands/explain.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 0d9663c..b2c39f5 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -492,6 +492,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
/* Create textual dump of plan tree */
ExplainPrintPlan(es, queryDesc);
+ /* COSTS OFF is used for regression tests - don't print the plan time */
if (es->costs && planduration)
{
double plantime = INSTR_TIME_GET_DOUBLE(*planduration);
@@ -525,7 +526,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
totaltime += elapsed_time(&starttime);
- if (es->analyze)
+ /* COSTS OFF is used for regression tests - don't print the execution time */
+ if (es->costs && es->analyze)
{
if (es->format == EXPLAIN_FORMAT_TEXT)
appendStringInfo(es->str, "Execution time: %.3f ms\n",
--
2.0.0.rc2.4.g1dc51c6.dirty
0003-Add-tests-for-interaction-between-visibility-and-CRE.patchtext/x-patch; charset=us-asciiDownload
>From 7f04ac6255360f21dcba0323630fbb4b85bb6cab Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 4 Jun 2014 21:36:19 +0200
Subject: [PATCH 3/3] Add tests for interaction between visibility and CREATE
INDEX's heap scan.
There have been several bugs over the years in the interaction between
HeapTupleSatisfiesVacuum() and CREATE INDEX. It's also looking like
there will be some changes to the visiblity routines expected during
the 9.5 cycle.
Those facts together seem to warrant a couple of tests, even if they
aren't exactly pretty.
---
src/test/regress/expected/create_index.out | 198 +++++++++++++++++++++++++++++
src/test/regress/sql/create_index.sql | 65 ++++++++++
2 files changed, 263 insertions(+)
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index f6f5516..b9cd3d8 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2782,3 +2782,201 @@ explain (costs off)
Index Cond: ((thousand = 1) AND (tenthous = 1001))
(2 rows)
+----
+-- Check interactions of index creation in a xact that also has done
+-- insertions, updates and deletions. There've been several bugs
+-- around that.
+----
+CREATE TABLE sametrans1(a text, b int default 1);
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-del');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-del-after-index');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-main');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-sub-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-sub');
+BEGIN;
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-subdel-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-subdel');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-upd-sub-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-upd-sub');
+DELETE FROM sametrans1 WHERE a = 'ins-before-xact-del';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-main';
+SAVEPOINT a;
+INSERT INTO sametrans1(a) VALUES ('ins-sub');
+INSERT INTO sametrans1(a) VALUES ('ins-sub-del-mainxact');
+DELETE FROM sametrans1 WHERE a = 'ins-mainxact-subdel';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-sub';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-mainxact-upd-sub';
+RELEASE SAVEPOINT a;
+SAVEPOINT b;
+INSERT INTO sametrans1(a) VALUES ('ins-sub-abort');
+DELETE FROM sametrans1 WHERE a = 'ins-mainxact-subdel-abort';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-sub-abort';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-mainxact-upd-sub-abort';
+ROLLBACK TO b;
+-- don't add anything here, page pruning might otherwise remove rows
+CREATE UNIQUE INDEX ON sametrans1 (a);
+CREATE INDEX ON sametrans1 (b);
+DELETE FROM sametrans1 WHERE a = 'ins-sub-del-mainxact';
+DELETE FROM sametrans1 WHERE a = 'ins-before-xact-del-after-index';
+COMMIT;
+INSERT INTO sametrans1(a) VALUES ('ins-after-xact');
+-- force bitmapscans, they show the indexscans separately from the heap scan
+SET enable_seqscan = off;
+SET enable_indexscan = off;
+SET enable_indexonlyscan = off;
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-before-xact'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-del';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=0 loops=1)
+ Recheck Cond: (a = 'ins-before-xact-del'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact-del'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-del-after-index';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=0 loops=1)
+ Recheck Cond: (a = 'ins-before-xact-del-after-index'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact-del-after-index'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-main';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-before-xact-upd-main'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact-upd-main'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-sub-abort';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-before-xact-upd-sub-abort'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact-upd-sub-abort'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-sub';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-before-xact-upd-sub'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-before-xact-upd-sub'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-mainxact'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-mainxact'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-subdel-abort';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-mainxact-subdel-abort'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-mainxact-subdel-abort'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-subdel';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=0 loops=1)
+ Recheck Cond: (a = 'ins-mainxact-subdel'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-mainxact-subdel'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-upd-sub-abort';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-mainxact-upd-sub-abort'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-mainxact-upd-sub-abort'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-upd-sub';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-mainxact-upd-sub'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-mainxact-upd-sub'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-sub'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-sub'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub-del-mainxact';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=0 loops=1)
+ Recheck Cond: (a = 'ins-sub-del-mainxact'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-sub-del-mainxact'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub-abort';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=0 loops=1)
+ Recheck Cond: (a = 'ins-sub-abort'::text)
+ Heap Blocks:
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=0 loops=1)
+ Index Cond: (a = 'ins-sub-abort'::text)
+(5 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-after-xact';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Bitmap Heap Scan on sametrans1 (actual rows=1 loops=1)
+ Recheck Cond: (a = 'ins-after-xact'::text)
+ Heap Blocks: exact=1
+ -> Bitmap Index Scan on sametrans1_a_idx (actual rows=1 loops=1)
+ Index Cond: (a = 'ins-after-xact'::text)
+(5 rows)
+
+SET enable_seqscan = on;
+SET enable_indexscan = on;
+SET enable_indexonlyscan = on;
+DROP TABLE sametrans1;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index d4d24ef..1fae3e9 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -938,3 +938,68 @@ ORDER BY thousand;
explain (costs off)
select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null));
+
+----
+-- Check interactions of index creation in a xact that also has done
+-- insertions, updates and deletions. There've been several bugs
+-- around that.
+----
+CREATE TABLE sametrans1(a text, b int default 1);
+
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-del');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-del-after-index');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-main');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-sub-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-before-xact-upd-sub');
+BEGIN;
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-subdel-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-subdel');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-upd-sub-abort');
+INSERT INTO sametrans1(a) VALUES ('ins-mainxact-upd-sub');
+DELETE FROM sametrans1 WHERE a = 'ins-before-xact-del';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-main';
+SAVEPOINT a;
+INSERT INTO sametrans1(a) VALUES ('ins-sub');
+INSERT INTO sametrans1(a) VALUES ('ins-sub-del-mainxact');
+DELETE FROM sametrans1 WHERE a = 'ins-mainxact-subdel';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-sub';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-mainxact-upd-sub';
+RELEASE SAVEPOINT a;
+SAVEPOINT b;
+INSERT INTO sametrans1(a) VALUES ('ins-sub-abort');
+DELETE FROM sametrans1 WHERE a = 'ins-mainxact-subdel-abort';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-before-xact-upd-sub-abort';
+UPDATE sametrans1 SET b = b + 1 WHERE a = 'ins-mainxact-upd-sub-abort';
+ROLLBACK TO b;
+-- don't add anything here, page pruning might otherwise remove rows
+CREATE UNIQUE INDEX ON sametrans1 (a);
+CREATE INDEX ON sametrans1 (b);
+DELETE FROM sametrans1 WHERE a = 'ins-sub-del-mainxact';
+DELETE FROM sametrans1 WHERE a = 'ins-before-xact-del-after-index';
+COMMIT;
+INSERT INTO sametrans1(a) VALUES ('ins-after-xact');
+-- force bitmapscans, they show the indexscans separately from the heap scan
+SET enable_seqscan = off;
+SET enable_indexscan = off;
+SET enable_indexonlyscan = off;
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-del';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-del-after-index';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-main';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-sub-abort';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-before-xact-upd-sub';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-subdel-abort';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-subdel';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-upd-sub-abort';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-mainxact-upd-sub';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub-del-mainxact';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-sub-abort';
+EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF) SELECT * FROM sametrans1 WHERE a = 'ins-after-xact';
+SET enable_seqscan = on;
+SET enable_indexscan = on;
+SET enable_indexonlyscan = on;
+DROP TABLE sametrans1;
--
2.0.0.rc2.4.g1dc51c6.dirty
Re: Andres Freund 2014-06-04 <20140604194544.GB785@awork2.anarazel.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 testNote 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@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Sep 23, 2014 at 1:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Tom Lane 2014-09-23 <15155.1411493559@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 24, 2014 at 8:02 AM, Christoph Berg <cb@df7cb.de> wrote:
Re: Tom Lane 2014-09-23 <15155.1411493559@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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@df7cb.de> wrote:
Re: Tom Lane 2014-09-23 <15155.1411493559@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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@df7cb.de> wrote:
Re: Tom Lane 2014-09-23 <15155.1411493559@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Sep 20, 2014 at 4:13 PM, Christoph Berg <cb@df7cb.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@dunklau.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Ronan Dunklau <ronan@dunklau.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Tom Lane 2014-10-12 <19766.1413129321@sss.pgh.pa.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@df7cb.de | http://www.df7cb.de/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-10-13 11:46:16 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Oct 12, 2014 at 11:55 AM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Oct 14, 2014 at 6:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@2ndquadrant.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@gmail.com> wrote:
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-10-16 10:06:59 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Oct 16, 2014 at 10:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml@gmail.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le jeudi 16 octobre 2014 10:43:25 Robert Haas a écrit :
On Thu, Oct 16, 2014 at 10:06 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Oct 14, 2014 at 3:03 AM, David Rowley <dgrowleyml@gmail.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.dunklau@dalibo.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andres Freund <andres@2ndquadrant.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@gmail.com> writes:
On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <dgrowleyml@gmail.com> writes:
On Fri, Oct 17, 2014 at 3:34 AM, Tom Lane <tgl@sss.pgh.pa.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 inthat
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 withsome
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 asExecution 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@gmail.com> writes:
On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-10-19 12:26:24 -0400, Tom Lane wrote:
David Rowley <dgrowleyml@gmail.com> writes:
On Sat, Oct 18, 2014 at 1:39 PM, Tom Lane <tgl@sss.pgh.pa.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-10-19 12:38:52 -0400, Tom Lane wrote:
Andres Freund <andres@2ndquadrant.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers