On Sun, 8 Jan 2023 at 23:21, Ankit Kumar Pandey <itsanki...@gmail.com> wrote: > Please find attached patch with addressed issues mentioned before.
Here's a quick review: 1. You can use foreach_current_index(l) to obtain the index of the list element. 2. I'd rather see you looping backwards over the list in the first loop. I think it'll be more efficient to loop backwards as you can just break out the loop when you see the pathkeys are not contained in the order by pathkreys. When the optimisation does not apply that means you only need to look at the last item in the list. You could maybe just invent foreach_reverse() for this purpose and put it in pg_list.h. That'll save having to manually code up the loop. 3. I don't think you should call the variable enable_order_by_pushdown. We've a bunch of planner related GUCs that start with enable_. That might cause a bit of confusion. Maybe just try_sort_pushdown. 4. You should widen the scope of orderby_pathkeys and set it within the if (enable_order_by_pushdown) scope. You can reuse this again in the 2nd loop too. Just initialise it to NIL 5. You don't seem to be checking all WindowClauses for a runCondtion. If you do #2 above then you can start that process in the initial reverse loop so that you've checked them all by the time you get around to that WindowClause again in the 2nd loop. 6. The test with "+-- Do not perform additional sort if column is presorted". I don't think "additional" is the correct word here. I think you want to ensure that we don't push down the ORDER BY below the WindowAgg for this case. There is no ability to reduce the sorts here, only move them around, which we agreed we don't want to do for this case. Also, do you want to have a go at coding up the sort bound pushdown too? It'll require removing the limitCount restriction and adjusting ExecSetTupleBound() to recurse through a WindowAggState. I think it's pretty easy. You could try it then play around with it to make sure it works and we get the expected performance. You'll likely want to add a few more rows than the last performance test you did or run the query with pgbench. Running a query once that only takes 1-2ms is likely not a reliable way to test the performance of something. David