On 2013-08-30 23:05:25 +0200, Andres Freund wrote:
> On 2013-08-30 23:00:15 +0200, Andres Freund wrote:
> > Hi,
> >
> >
> > On 2013-08-30 20:46:27 +0000, te...@elde.net wrote:
> > > I'm getting "out of binary heap slots", which offcourse spoils the fun of
> > > the query.
> >
> > > Explain analyze gives this plan (again anonymized a bit, but can send
> > > proper
> > > off-list):
> >
> > Since I reviewed the patch that introduced that message, I'd be
> > interested in getting that. Ideally in a state where I can reproduce the
> > issue in a new cluster.
>
> No need, found the bug. And I think can build a testcase myself.
>
> ExecReScanMergeAppend resets ms_initialized, but doesn't clear the
> binaryheap. Thus no new elements fit.
Ok, patch for that attached. Should we add
SELECT (SELECT g.i FROM ((SELECT random()::int ORDER BY 1 OFFSET 0) UNION ALL
(SELECT random()::int ORDER BY 1 OFFSET 0)) f(i) ORDER BY f.i LIMIT 1) FROM
generate_series(1, 10) g(i);
as a regression test? I slightly on the "no" side...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 1f6eae650b475dfe80868c78fcd805e3d49f0d71 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Fri, 30 Aug 2013 23:38:40 +0200
Subject: [PATCH] Reset the binary heap in merge append rescans
Failing to do so can cause queries to return wrong data, error out or crash.
This requires adding a new binaryheap_reset() method to the generic binaryheap
implementation.
Per bugreport from Terje Elde in bug #8410.
---
src/backend/executor/nodeMergeAppend.c | 1 +
src/backend/lib/binaryheap.c | 18 ++++++++++++++++--
src/include/lib/binaryheap.h | 1 +
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 5a48f7a..c3edd61 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -297,5 +297,6 @@ ExecReScanMergeAppend(MergeAppendState *node)
if (subnode->chgParam == NULL)
ExecReScan(subnode);
}
+ binaryheap_reset(node->ms_heap);
node->ms_initialized = false;
}
diff --git a/src/backend/lib/binaryheap.c b/src/backend/lib/binaryheap.c
index 4b4fc94..14fe24f 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/backend/lib/binaryheap.c
@@ -37,16 +37,30 @@ binaryheap_allocate(int capacity, binaryheap_comparator compare, void *arg)
sz = offsetof(binaryheap, bh_nodes) +sizeof(Datum) * capacity;
heap = palloc(sz);
- heap->bh_size = 0;
heap->bh_space = capacity;
- heap->bh_has_heap_property = true;
heap->bh_compare = compare;
heap->bh_arg = arg;
+ heap->bh_size = 0;
+ heap->bh_has_heap_property = true;
+
return heap;
}
/*
+ * binaryheap_reset
+ *
+ * Resets the heap to an empty state loosing it's ephemeral state, but not the
+ * parameters passed at allocation.
+ */
+void
+binaryheap_reset(binaryheap *heap)
+{
+ heap->bh_size = 0;
+ heap->bh_has_heap_property = true;
+}
+
+/*
* binaryheap_free
*
* Releases memory used by the given binaryheap.
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 1e99e72..85cafe4 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -40,6 +40,7 @@ typedef struct binaryheap
extern binaryheap *binaryheap_allocate(int capacity,
binaryheap_comparator compare,
void *arg);
+extern void binaryheap_reset(binaryheap *heap);
extern void binaryheap_free(binaryheap *heap);
extern void binaryheap_add_unordered(binaryheap *heap, Datum d);
extern void binaryheap_build(binaryheap *heap);
--
1.8.2.rc2.4.g7799588.dirty
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
