Re: Window function optimisation, allow pushdowns of items matching PARTITION BY clauses

From: Vik Fearing <vik(dot)fearing(at)dalibo(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: David Rowley <dgrowley(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Thomas Mayer <thomas(dot)mayer(at)student(dot)kit(dot)edu>
Subject: Re: Window function optimisation, allow pushdowns of items matching PARTITION BY clauses
Date: 2014-06-27 01:31:03
Message-ID: 53ACC957.8010409@dalibo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 06/27/2014 02:49 AM, Tom Lane wrote:
> Vik Fearing <vik(dot)fearing(at)dalibo(dot)com> writes:
>> This latest patch is ready for a committer to look at now. The weird
>> comments have been changed, superfluous regression tests removed, and
>> nothing done about expression pushdown per (brief) discussion.
>
> I started to look at this patch and realized that there's an issue that
> isn't covered, which is not too surprising because the existing code fails
> to cover it too. Remember that the argument for pushing down being safe
> at all is that we expect the pushed-down qual to yield the same result at
> all rows of a given partition, so that we either include or exclude the
> whole partition and thereby don't change window function results. This
> means that not only must the qual expression depend only on partitioning
> columns, but *it had better not be volatile*.
>
> In exactly the same way, it isn't safe to push down quals into
> subqueries that use DISTINCT unless the quals are non-volatile. This
> consideration is missed by the current code, and I think that's a bug.
>
> (Pushing down volatile quals would also be unsafe in subqueries involving
> aggregation, except that we put them into HAVING so that they're executed
> only once per subquery output row anyway.)

Are you going to take care of all this, or should David or I take a
crack at it? The commitfest app still shows Ready for Committer.

> Given the lack of prior complaints, I'm not excited about back-patching a
> change to prevent pushing down volatile quals in the presence of DISTINCT;
> but I think we probably ought to fix it in 9.5, and maybe 9.4 too.
>
> Thoughts?

I didn't test it myself, I'm just taking your word on it.

If it's a bug, it should obviously be fixed in 9.5. As for 9.4, I have
always viewed a beta as a time to fix bugs so I vote to backpatch it at
least that far.

Why wouldn't it go back all the way to 9.0? (assuming 8.4 is dead)
--
Vik

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-06-27 01:35:11 Re: Window function optimisation, allow pushdowns of items matching PARTITION BY clauses
Previous Message Ian Barwick 2014-06-27 00:52:57 Re: "RETURNING PRIMARY KEY" syntax extension