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
da...@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
