Hello Ashutosh, Thank you for your email and for reviewing the patch. I sincerely apologize for the delay in responding.
On Wed, Mar 6, 2024 at 11:16 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > here's summary of observations > 1. The patch improves planning time significantly (3X to 20X) and the > improvement increases with the number of tables being joined. > 2. In the assert enabled build the patch slows down (in comparison to HEAD) > planning with higher number of tables in the join. You may want to > investigate this. But this is still better than my earlier measurements. > 3. The patch increased memory consumption by planner. But the numbers have > improved since my last measurement. Still it will be good to investigate what > causes this extra memory consumption. > 4. Generally with the assert enabled build planner consumes more memory with > or without patch. From my previous experience this might be due to Bitmapset > objects created within Assert() calls. Thank you for testing the patch and sharing the results. For comment #1, these results show the effectiveness of the patch. For comment #2, I agree that we should not slow down assert-enabled builds. The patch adds a lot of assertions to avoid adding bugs, but they might be too excessive. I will reconsider these assertions and remove unnecessary ones. For comments #3 and #4, while the patch improves time complexity, it has some negative impacts on space complexity. The patch uses a Bitmapset-based index to speed up searching for EquivalenceMembers and RestrictInfos. Reducing this memory consumption is a little hard, but this is a very important problem in committing this patch, so I will investigate this further. > Does v24-0002 have any relation/overlap with my patches to reduce memory > consumed by RestrictInfos? Those patches have code to avoid creating > duplicate RestrictInfos (including commuted RestrictInfos) from ECs. [2] Thank you for sharing these patches. My patch may be related to your patches. My patch speeds up slow linear searches over EquivalenceMembers and RestrictInfos. It uses several approaches, one of which is the Bitmapset-based index. Bitmapsets are space inefficient, so if there are many EquivalenceMembers and RestrictInfos, this index becomes large. This is true for highly partitioned cases, where there are a lot of similar (or duplicate) elements. Eliminating such duplicate elements may help my patch reduce memory consumption. I will investigate this further. Unfortunately, I've been busy due to work, so I may not be able to respond soon. I really apologize for this. However, I will look into the patches, including yours, and share further information if found. Again, I apologize for my late response and appreciate your kind review. -- Best regards, Yuya Watari