Afaiac we should add a simple testcase here, like I suggested in 477344d5f17c4a8e95d3a5bb6642718a. Apart from that I am not sure there is work to be done here.
Am I wrong?
Regards
Arne
From: Arne Roland <A.Roland@index.de> Sent: Saturday, June 26, 2021 5:50:49 PM To: Tomas Vondra Cc: pgsql-hackers Subject: Re: PATCH: generate fractional cheapest paths in generate_orderedappend_path
Hi Tomas,
I don't think there is much work left to do here.
Did you have a look at the test case? Did it make sense to you?
And I am sorry. I had another look at this and it seems I was confused (again).
From: Arne Roland Sent: Monday, April 26, 2021 13:00 To: Tomas Vondra; pgsql-hackers Subject: Re: PATCH: generate fractional cheapest paths in generate_orderedappend_path
> I think it should. We have a ParallelAppend node after all.
> I'm not really familiar with the way get_cheapest_fractional_path_for_pathkeys is used, but a quick search suggests to
> me, that build_minmax_path was thus far the only one using it. And minmax paths are never parallel safe anyway. I think that is the reason it doesn't do that already.
The whole segment were are talking about obviously assumes require_parallel_safe is not needed. I wasn't aware that in set_append_rel_size. And I just realized there is a great comment explaining why it rightfully does so:
/* * If any live child is not parallel-safe, treat the whole appendrel * as not parallel-safe. In future we might be able to generate plans * in which some children are farmed out to workers while others are * not; but we don't have that today, so it's a waste to consider * partial paths anywhere in the appendrel unless it's all safe. * (Child rels visited before this one will be unmarked in * set_append_rel_pathlist().) */
So afaik we don't need to think further about this.
From: Tomas Vondra <tomas.vondra@enterprisedb.com> Sent: Thursday, June 3, 2021 22:57 To: Zhihong Yu Cc: Arne Roland; pgsql-hackers Subject: Re: PATCH: generate fractional cheapest paths in generate_orderedappend_path > Actually, there are two comments > > /* XXX maybe we should have startup_new_fractional? */ > > in the patch I posted - I completely forgot about that. But I think > that's a typo, I think - it should be > > /* XXX maybe we should have startup_neq_fractional? */ > > and the new flag would work similarly to startup_neq_total, i.e. it's > pointless to add paths where startup == fractional cost. > > At least I think that was the idea when I wrote the patch, it way too > long ago.
> Sorry, I almost forgot about this myself. I only got reminded upon seeing that again with different queries/tables.
> Just to be sure I get this correctly: You mean startup_gt_fractional (cost) as an additional condition, right?