On 06/27/2014 02:49 AM, Tom Lane wrote:
> Vik Fearing <vik.fearing@dalibo.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
