From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: James Coleman <jtc331@gmail.com>
Cc: Jaime Casanova <jaime.casanova@2ndquadrant.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: enable_incremental_sort changes query behavior
Date: Thu, 8 Oct 2020 00:34:38 +0200
Message-ID: <20201007223438.npgyo4qqhzkztdnd@development> (raw)
In-Reply-To: <CA+Tgmoa3oNvLBio62jSGXCRhOotsofiqn9guus3+NTQvz6qPWA@mail.gmail.com>
References: <CAJGNTeNaxpXgBVcRhJX+2vSbq+F2kJqGBcvompmpvXb7pq+oFA@mail.gmail.com>
<CAAaqYe_Ru5h2NxhGtERfj1RGskyjAg8PfG1XYTLRYiUmqNGEXw@mail.gmail.com>
<CAJGNTeN4iqAAjUn1UJJR-Yhat-B7zsKkj50KN-AYzkE5fStXvw@mail.gmail.com>
<CAAaqYe9QBf_UWT+-_SrMs4iHKbUC_WkaHvJjthJuqrmvW2UdZw@mail.gmail.com>
<CA+Tgmoa3oNvLBio62jSGXCRhOotsofiqn9guus3+NTQvz6qPWA@mail.gmail.com>
On Wed, Oct 07, 2020 at 03:52:04PM -0400, Robert Haas wrote:
>On Thu, Oct 1, 2020 at 9:03 AM James Coleman <jtc331@gmail.com> wrote:
>> The plan (obtained by replacing the volatile function with a stable one):
>>
>> Unique
>> -> Nested Loop
>> -> Gather Merge
>> Workers Planned: 2
>> -> Sort
>> Sort Key: ref_0.i, (md5(ref_0.t))
>> -> Parallel Index Scan using ref_0_i_idx on ref_0
>> -> Function Scan on generate_series ref_1
>>
>> Changing `md5(ref_0.t)` to `random()::text || ref_0.t` causes the error.
>
>As far as I remember this stuff, target lists for any nodes below the
>top of the join tree were previously always just Var nodes. Any
>expressions that needed to be computed were computed at the level of
>the topmost join, before adding upper planner steps like Agg,
>WindowAgg, Limit, etc. But, here you've got a complex expression --
>md5(ref_0.t) -- begin computed somewhere in the middle of the plan
>tree so that the sort can be performed at that point. However, that
>seems likely to break a bunch of assumptions all over the planner,
>because, unless a lot of work and surgery has been done since I looked
>at this last, that md5(ref_0.t) value is not going to "bubble up" to
>higher levels of the plan through the tlists of the individual nodes.
>
>That poses a risk that it will be recomputed multiple times, which is
>particularly bad for volatile functions because you might not always
>get the same answer, but it seems like it might be bad even for
>non-volatile functions, because it could be slow if the value is used
>at multiple places in the query. It feels like the sort of thing that
>might have broken some other assumptions, too, although I can't say
>exactly what.
I agree this might be a clear correctness issue for volatile functions.
Not sure about stable functions, though - it's easy to construct cases
where pushing the function down results in much faster execution.
Consider this:
create table t (id int, val text);
insert into t select 1, repeat('x', 1000) from generate_series(1,100) s(i);
-- no pushdown
select id, md5(t1.val) from t t1 join t t2 using (id) join t t3 using (id);
Time: 5222.895 ms (00:05.223)
-- manual pushdown
with x as (select id, md5(t.val) from t)
select id, t1.md5 from x t1 join x t2 using (id) join x t3 using (id);
Time: 486.455 ms
Of course, this is independent of what the planner assumes correctly,
which is what's being discussed in this thread. And I'm sure there are
plenty counter-examples too, so we'd need to cost this properly.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, robertmhaas@gmail.com, jtc331@gmail.com, jaime.casanova@2ndquadrant.com
Subject: Re: enable_incremental_sort changes query behavior
In-Reply-To: <20201007223438.npgyo4qqhzkztdnd@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox