Re: [PoC] Reducing planning time when tables have many partitions

Hello,

On Fri, Jan 27, 2023 at 12:48 PM Yuya Watari <watari(dot)yuya(at)gmail(dot)com> wrote:
> However, there is more bad news. Unfortunately, some regression tests
> are failing in my environment. I'm not sure why, but it could be that
> a) my sanity checking code (v12-0004) is wrong, or b) our patches have
> some bugs.
>
> I will investigate this issue further, and share the results when found.

I have investigated this issue and concluded that b) our patches have
some bugs. I have attached the modified patches to this email. This
version passed regression tests in my environment.

1. v13-0005

The first bug is in eclass_member_iterator_strict_next(). As I
mentioned in the commit message, the original code incorrectly missed
EquivalenceMembers with empty em_relids when 'with_norel_members' is
true.

I show my changes as follows:

===
- if (!iter->with_children && em->em_is_child)
- continue;

- if (!iter->with_norel_members && bms_is_empty(em->em_relids))
- continue;

- if (!bms_is_subset(iter->with_relids, em->em_relids))
- continue;

- iter->current_index = foreach_current_index(lc);
+ if ((iter->with_norel_members && bms_is_empty(em->em_relids))
+ || (bms_is_subset(iter->with_relids, em->em_relids)
+ && (iter->with_children || !em->em_is_child)))
+ {
+ iter->current_index = foreach_current_index(lc);
===

EquivalenceMembers with empty em_relids will pass the second 'if'
condition when 'with_norel_members' is true. These members should be
returned. However, since the empty em_relids can never be superset of
any non-empty relids, the EMs may fail the last condition. Therefore,
the original code missed some members.

2. v13-0006

The second bug exists in get_ecmember_indexes_strict(). As I described
in the comment, if the empty relids is given, this function must
return all members because their em_relids are always superset. I am
concerned that this change may adversely affect performance.
Currently, I have not seen any degradation.

3. v13-0007

The last one is in add_eq_member(). I am not sure why this change is
working, but it is probably related to the concerns David mentioned in
the previous mail. The v13-0007 may be wrong, so it should be
reconsidered.

--
Best regards,
Yuya Watari