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 <and...@anarazel.de> > 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 <amitlangot...@gmail.com> 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