Re: BRIN cost estimate

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: 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-26 11:44:42
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

> 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?

Attachment Content-Type Size
brin-correlation-v5.patch application/octet-stream 17.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-03-26 14:00:15 Re: logical replication apply to run with sync commit off by default
Previous Message Thomas Munro 2017-03-26 11:39:49 Re: delta relations in AFTER triggers