Hello David,
sorry for the late response. I will try out your changes from the view
of a user in mid-June. However, I can't do a trustworthy code review as
I'm not an experienced postgre-hacker (yet).
Best Regards
Thomas
Am 14.04.2014 13:19, schrieb David Rowley:
On 14 April 2014 03:31, Tom Lane <t...@sss.pgh.pa.us
<mailto:t...@sss.pgh.pa.us>> wrote:
David Rowley <dgrow...@gmail.com <mailto:dgrow...@gmail.com>> writes:
> On this thread
>
http://www.postgresql.org/message-id/52c6f712.6040...@student.kit.edu there
> was some discussion around allowing push downs of quals that
happen to be
> in every window clause of the sub query. I've quickly put
together a patch
> which does this (see attached)
I think you should have check_output_expressions deal with this,
instead.
Compare the existing test there for non-DISTINCT output columns.
I've moved the code there and it seems like a much better place for it.
Thanks for the tip.
> Oh and I know that my function
var_exists_in_all_query_partition_by_clauses
> has no business in allpaths.c, I'll move it out as soon as I find
a better
> home for it.
I might be wrong, but I think you could just embed that search loop in
check_output_expressions, and it wouldn't be too ugly.
I've changed the helper function to take the TargetEntry now the same
as targetIsInSortList does for the hasDistinctOn test and I renamed the
helper function to targetExistsInAllQueryPartitionByClauses. It seems
much better, but there's probably a bit of room for improving on the
names some more.
I've included the updated patch with some regression tests.
The final test I added tests that an unused window which would, if used,
cause the pushdown not to take place still stops the pushdownf from
happening... I know you both talked about this case in the other thread,
but I've done nothing for it as Tom mentioned that this should likely be
fixed elsewhere, if it's even worth worrying about at all. I'm not quite
sure if I needed to bother including this test for it, but I did anyway,
it can be removed if it is deemed unneeded.
Per Thomas' comments, I added a couple of tests that ensure that the
order of the columns in the partition by clause don't change the
behaviour. Looking at the implementation of the actual code makes this
test seems a bit unneeded as really but I thought that the test should
not assume anything about the implementation so from that point of view
the extra test seems like a good idea.
For now I can't think of much else to do for the patch, but I'd really
appreciate it Thomas if you could run it through the paces if you can
find any time for it, or maybe just give my regression tests a once over
to see if you can think of any other cases that should be covered.
Regards
David Rowley
--
======================================
Thomas Mayer
Durlacher Allee 61
D-76131 Karlsruhe
Telefon: +49-721-2081661
Fax: +49-721-72380001
Mobil: +49-174-2152332
E-Mail: thomas.ma...@student.kit.edu
=======================================
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
