> 1 марта 2018 г., в 22:44, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
>
> Michail Nikolaev <michail.nikolaev@gmail.com> writes:
>> I have added small change to patch to allow it be compiled using msvc (uint64_t -> uint64).
>> Everything seems to work, check-world is passing.
>
>> Actually patch fixes two issues:
>> 1) Partial GIST indexes now have corrent tuples count estimation.
>> 2) Now subsequent calls to VACUUM on GIST index (like "VACCUM table_name") do not change tuples count to estimated
numberof tuples in table (which is changed even without any updates in table due current implementation).
>
>> I think it is fine to commit.
>
> I took a quick look at this. I wonder what is the point of making
> the counting conditional. Since the function is visiting every
> index page anyway, why not just always count and unconditionally
> provide an exact answer? The number of cycles saved by skipping
> "tuplesCount += PageGetMaxOffsetNumber(page)" on each leaf page
> is surely trivial.
Thanks for looking into the patch, Tom!
I thought that it's a good idea to optimize out as many cycles as possible.
But, indeed, there are some reasons in favor of unconditional counting:
1. Code is cleaner, and this is not hot path
2. If we choose unconditional counting in gistvacuumcleanup() I'll remove those counting cycles in gistbulkdelete() in
mainvacuum patch (for v12). Both functions will have less code.
So, I agree, unconditional counting is a good idea. Here's the v3 patch.
Best regards, Andrey Borodin.