On 7/8/2023 15:19, Yuya Watari wrote:
Hello,
Thank you for your reply.
On Thu, Aug 3, 2023 at 10:29 PM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
If you think that the verification is important to catch bugs, you may want to
encapsulate it with an #ifdef .. #endif such that the block within is not
compiled by default. See OPTIMIZER_DEBUG for example.
In my opinion, verifying the iteration results is only necessary to
avoid introducing bugs while developing this patch. The verification
is too excessive for regular development of PostgreSQL. I agree that
we should avoid a significant degradation in assert enabled builds, so
I will consider removing it.
I should admit, these checks has helped me during backpatching this
feature to pg v.13 (users crave speed up of query planning a lot). Maybe
it is a sign of a lack of tests, but in-fact, it already has helped.
One more thing: I think, you should add comments to
add_child_rel_equivalences() and add_child_join_rel_equivalences()
on replacing of:
if (bms_is_subset(cur_em->em_relids, top_parent_relids) &&
!bms_is_empty(cur_em->em_relids))
and
if (bms_overlap(cur_em->em_relids, top_parent_relids))
with different logic. What was changed? It will be better to help future
developers realize this part of the code more easily by adding some
comments.
Do you think that the memory measurement patch I have shared in those threads
is useful in itself? If so, I will start another proposal to address it.
For me, who is developing the planner in this thread, the memory
measurement patch is useful. However, most users do not care about
memory usage, so there is room for consideration. For example, making
the metrics optional in EXPLAIN ANALYZE outputs might be better.
+1. Any memory-related info in the output of EXPLAIN ANALYZE makes tests
more complex because of architecture dependency.
--
regards,
Andrey Lepikhov
Postgres Professional
