Re: On disable_cost

Hi!
On 04.10.2024 00:52, David Rowley wrote:
> One thing the patch did cause me to find is the missing propagation of
> disabled_nodes in make_sort(). It was very obviously wrong with the
> patched EXPLAIN output and wasn't so obvious with the current output,
> so perhaps you could look at this patch as a better way of ensuring
> the disable_node propagation is correct. That's much harder logic to
> get right than what I've added to explain.c as it's spread out in many
> places.
>
> Take this case, for example:
>
> create table lp (a int) partition by list(a);
> create table lp1 partition of lp for values in(1);
> create table lp2 partition of lp for values in(2);
> create index on lp1(a);
> insert into lp select 1 from generate_Series(1,1000000);
> analyze lp;
> set enable_sort=0;
> explain (costs off) select * from lp order by a;
>
> master gives:
>
> Append
> Disabled Nodes: 1
> -> Index Only Scan using lp1_a_idx on lp1 lp_1
> -> Sort
> Sort Key: lp_2.a
> -> Seq Scan on lp2 lp_2
>
> which isn't correct. Append appears disabled, but it's not. Sort is.
> Before I fixed that in the patch, I was incorrectly getting the
> "Disabled: true" under the Append. I feel we're more likely to get bug
> reports alerting us to incorrect logic when the disabled property only
> appears on disabled nodes as there are far fewer of them to look at
> and therefore it's more obvious when they're misplaced.
>
> The patched version correctly gives us:
>
> Append
> -> Index Only Scan using lp1_a_idx on lp1 lp_1
> -> Sort
> Disabled: true
> Sort Key: lp_2.a
> -> Seq Scan on lp2 lp_2

To be honest, I don’t understand at all why we don’t count disabled
nodes for append here? As I understand it, this is due to the fact that
the partitioned table can also be scanned by an index. Besides
mergeappend, in general it’s difficult for me to generalize for which
nodes this rule applies, can you explain here?

--
Regards,
Alena Rybakina
Postgres Professional