Re: [WIP] [B-Tree] Retail IndexTuple deletion

On Wed, Jun 27, 2018 at 12:10 PM, Andrey V. Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
>
>
> On 23.06.2018 00:43, Peter Geoghegan wrote:
>>
>> On Fri, Jun 22, 2018 at 4:24 AM, Andrey V. Lepikhov
>> <a.lepikhov@postgrespro.ru> wrote:
>>>
>>> According to your feedback, i develop second version of the patch.
>>> In this version:

I had quick look at your patch, I have few comments.
1.
+ if (use_quick_strategy)
+ quick_vacuum_index(Irel[i], onerel, vacrelstats);
+ else
+ lazy_vacuum_index(Irel[i],

I noticed that inside quick_vacuum_index again we check if we don't
have am routine for the target
delete then we call lazy_vacuum_index.  I think we can have that check
here itself?

2.
+ else
+ {
+ IndexBulkDeleteResult **stats;
+
+ lazy_vacuum_index(irel, stats, vacrelstats);
+ }

These stats will be lost, I think if you fix first then this will
issue will be solved, otherwise, you might
need to pass indexstats as a parameter to quick_vacuum_index function.

3.
+}
+
+#include "access/nbtree.h"
+#include "catalog/index.h"
+#include "executor/executor.h"

You can move these header inclusions at the top of the file.

4. Many places PG coding standard is not followed, few examples
a.
+ bool use_quick_strategy =
(vacrelstats->num_dead_tuples/vacrelstats->old_live_tuples <
target_index_deletion_factor);
+
space between operator and variable
vacrelstats->num_dead_tuples/vacrelstats->old_live_tuples  ->
vacrelstats->num_dead_tuples / vacrelstats->old_live_tuples

b.qsort((void *)vacrelstats  -> qsort((void *) vacrelstats

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com