Re: segfault with incremental sort

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: luis(dot)roberto(at)siscobra(dot)com(dot)br
Cc: pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>, "alan(dot)formagi" <alan(dot)formagi(at)siscobra(dot)com(dot)br>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, James Coleman <jtc331(at)gmail(dot)com>
Subject: Re: segfault with incremental sort
Date: 2020-11-03 16:52:06
Message-ID: 924226.1604422326@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

luis(dot)roberto(at)siscobra(dot)com(dot)br writes:
> It took me some time to make it the same... I managed to simplify the error. It appears to be something related to a subplan with a distinct clause and another subplan...

Thanks for the test case! It looks like this issue is somewhat related
to the "enable_incremental_sort changes query behavior" thread [1].
What I see happening is

1. The SELECT DISTINCT gives rise to a sort key expression that
contains non-parallel-safe SubPlans. (It's not immediately apparent
to me why we don't consider these particular subqueries parallel safe,
but they aren't. Anyway such a situation surely has to be allowed for.)

2. The planner ignores the fact that the sort key isn't parallel-safe
and makes a plan with IncrementalSort below Gather anyway. (I'm not
certain that this bug is specific to IncrementalSort; but given the lack
of previous complaints, I'm guessing we avoid this somehow for regular
sorts.)

3. ExecSerializePlan notes that the subplans aren't parallel-safe and
doesn't send them to the workers.

4. In the workers, nodeSubplan.c dumps core because the planstate it's
expecting is not there.

So, not only do we need to be thinking about volatility while checking
whether IncrementalSort is possible, but also parallel-safety.

In the meantime, now that I've seen this I don't have a lot of confidence
that we'll never inject similar bugs in future. I'm thinking of
committing the attached to at least reduce the stakes from "core dump"
to "weird error message".

regards, tom lane

[1] https://www.postgresql.org/message-id/flat/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com

Attachment Content-Type Size
more-paranoia-in-ExecInitSubPlan.patch text/x-diff 790 bytes

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message luis.roberto 2020-11-03 17:06:08 Re: segfault with incremental sort
Previous Message Tom Lane 2020-11-03 16:22:59 Re: User with BYPASSRLS privilege can't change password