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.