On Wed, Feb 27, 2019 at 7:46 AM Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: > Yet another thing I noticed while working on [1] is this in > grouping_planner: > > /* > * If the input rel is marked consider_parallel and there's nothing > that's > * not parallel-safe in the LIMIT clause, then the final_rel can be > marked > * consider_parallel as well. Note that if the query has rowMarks or is > * not a SELECT, consider_parallel will be false for every relation > in the > * query. > */ > if (current_rel->consider_parallel && > is_parallel_safe(root, parse->limitOffset) && > is_parallel_safe(root, parse->limitCount)) > final_rel->consider_parallel = true; > > If there is a need to add a LIMIT node, we don't consider generating > partial paths for the final relation below (see commit > 0927d2f46ddd4cf7d6bf2cc84b3be923e0aedc52), so it seems unnecessary > anymore to assess the parallel-safety of the LIMIT and OFFSET clauses. > To save cycles, why not remove those tests from that function like the > attached?
Because in the future we might want to consider generating partial_paths in cases where we don't do so today. I repeatedly made the mistake of believing that I could not bother setting consider_parallel entirely correctly for one reason or another, and I've gone through multiple iterations of fixing cases where I did that and it turned out to cause problems. I now believe that we should try to get it right in every case, whether or not we currently think it's possible for it to matter. Sometimes it matters in ways that aren't obvious, and it complicates further development. I don't think we'd save much by changing this test anyway. Those is_parallel_safe() tests aren't entirely free, of course, but they should be very cheap. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
