From: | David Steele <david(at)pgmasters(dot)net> |
---|---|
To: | emre(at)hasegeli(dot)com, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: BRIN cost estimate |
Date: | 2017-03-31 14:37:25 |
Message-ID: | cacba563-a461-b750-fa26-525438263978@pgmasters.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 3/26/17 7:44 AM, Emre Hasegeli wrote:
>> If we want to have a variable which stores the number of ranges, then
>> I think numRanges is better than numBlocks. I can't imagine many
>> people would disagree there.
>
> I renamed it with other two.
>
>> At the very least please write a comment to explain this in the code.
>> Right now it looks broken. If I noticed this then one day in the
>> future someone else will. If you write a comment then person of the
>> future will likely read it, and then not raise any questions about the
>> otherwise questionable code.
>
> I added a sentence about it.
>
>> I do strongly agree that the estimates need improved here. I've
>> personally had issues with bad brin estimates before, and I'd like to
>> see it improved. I think the patch is not quite complete without it
>> also considering stats on expression indexes. If you have time to go
>> do that I'd suggest you go ahead with that.
>
> I copy-pasted expression index support from btcostestimate() as well,
> and extended the regression test.
>
> I think this function can use more polishing before committing, but I
> don't know where to begin. There are single functions for every
> access method in here. I don't like them being in there to begin
> with. selfuncs.s is quite long with all kinds of dependencies and
> dependents. I think it would be better to have the access method
> selectivity estimation functions under src/access. Maybe we should
> start doing so by moving this to src/access/brin/brin_selfuncs.c.
> What do you think?
I have marked this patch "Needs Review".
--
-David
david(at)pgmasters(dot)net
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2017-03-31 14:44:11 | Re: GUC for cleanup indexes threshold. |
Previous Message | Pavel Stehule | 2017-03-31 14:34:05 | Re: Variable substitution in psql backtick expansion |