On 2018/06/13 16:35, Amit Langote wrote:
> Fwiw, I see that the crash can also occur even when using a
> non-partitioned table in the query, as shown in the following example
> which reuses Rajkumar's test data and query:
>
> create table foo (a int, b int, c text);
> postgres=# insert into foo select i%20, i%30, to_char(i%12, 'FM0000') from
> generate_series(0, 36) i;
>
> select dense_rank(b) within group (order by a) from foo group by b order by 1;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> Following query in the regression test suite can also be made to crash by
> adding a group by clause:
>
> select dense_rank(3) within group (order by x) from (values
> (1),(1),(2),(2),(3),(3),(4)) v(x) group by (x);
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
>
> Looking at the core dump of this, it seems the following commit may be
> relevant:
>
> commit bf6c614a2f2c58312b3be34a47e7fb7362e07bcb
> Author: Andres Freund <[email protected]>
> Date: Thu Feb 15 21:55:31 2018 -0800
>
> Do execGrouping.c via expression eval machinery, take two.
I studied this a bit and found a bug that's causing the crash.
The above mentioned commit has this hunk:
@@ -1309,6 +1311,9 @@ hypothetical_dense_rank_final(PG_FUNCTION_ARGS)
PG_RETURN_INT64(rank);
osastate = (OSAPerGroupState *) PG_GETARG_POINTER(0);
+ econtext = osastate->qstate->econtext;
+ if (!econtext)
+ osastate->qstate->econtext = econtext =
CreateStandaloneExprContext();
In CreateStandloneExprContext(), we have this:
econtext->ecxt_per_query_memory = CurrentMemoryContext;
/*
* Create working memory for expression evaluation in this context.
*/
econtext->ecxt_per_tuple_memory =
AllocSetContextCreate(CurrentMemoryContext,
"ExprContext",
ALLOCSET_DEFAULT_SIZES);
I noticed when debugging the crashing query that CurrentMemoryContext is
actually per-tuple memory context of some expression context of the
calling code, which would get reset before getting here again. So, it's
wrong of hypothetical_dense_rank_final to call CreateStandloneExprContext
without first switching to an actual per-query context.
Attached patch seems to fix the crash.
Thanks,
Amit
>From d1365b84ccf7c1028fca5f7056781b2fc95093a1 Mon Sep 17 00:00:00 2001
From: amit <[email protected]>
Date: Mon, 2 Jul 2018 16:29:19 +0900
Subject: [PATCH v1] Set correct memory context in
hypothetical_dense_rank_final
One of the memory context that it caches in OSAPerGroupState, which
is passed to it on every call, is parented by a memory context of
wrong life time. Fix that by switching to a per-query context before
creating the aforementioned context.
---
src/backend/utils/adt/orderedsetaggs.c | 8 ++++++++
src/test/regress/expected/aggregates.out | 9 +++++++++
src/test/regress/sql/aggregates.sql | 3 +++
3 files changed, 20 insertions(+)
diff --git a/src/backend/utils/adt/orderedsetaggs.c
b/src/backend/utils/adt/orderedsetaggs.c
index ed36851fdd..a758748cc7 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -1310,7 +1310,15 @@ hypothetical_dense_rank_final(PG_FUNCTION_ARGS)
osastate = (OSAPerGroupState *) PG_GETARG_POINTER(0);
econtext = osastate->qstate->econtext;
if (!econtext)
+ {
+ MemoryContext qcontext = osastate->qstate->qcontext;
+ MemoryContext oldcontext;
+
+ /* Make sure to we create econtext under correct parent
context. */
+ oldcontext = MemoryContextSwitchTo(qcontext);
osastate->qstate->econtext = econtext =
CreateStandaloneExprContext();
+ MemoryContextSwitchTo(oldcontext);
+ }
/* Adjust nargs to be the number of direct (or aggregated) args */
if (nargs % 2 != 0)
diff --git a/src/test/regress/expected/aggregates.out
b/src/test/regress/expected/aggregates.out
index 10d6bb6824..a120dd83f7 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2092,3 +2092,12 @@ SELECT variance(unique1::int4), sum(unique1::int8) FROM
tenk1;
(1 row)
ROLLBACK;
+-- test coverage for dense_rank
+SELECT dense_rank(x) WITHIN GROUP (ORDER BY x) FROM (VALUES
(1),(1),(2),(2),(3),(3)) v(x) GROUP BY (x) ORDER BY 1;
+ dense_rank
+------------
+ 1
+ 1
+ 1
+(3 rows)
+
diff --git a/src/test/regress/sql/aggregates.sql
b/src/test/regress/sql/aggregates.sql
index 1b6db50956..7e77467ecd 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -925,3 +925,6 @@ EXPLAIN (COSTS OFF)
SELECT variance(unique1::int4), sum(unique1::int8) FROM tenk1;
ROLLBACK;
+
+-- test coverage for dense_rank
+SELECT dense_rank(x) WITHIN GROUP (ORDER BY x) FROM (VALUES
(1),(1),(2),(2),(3),(3)) v(x) GROUP BY (x) ORDER BY 1;
--
2.11.0
