Re: [HACKERS] BRIN cost estimate

2017-04-06 Thread Alvaro Herrera
Tom Lane wrote: > TBH, I think that code is in the noise. It doesn't involve any disk > access, or catalog access, or user-defined function calls. I wouldn't > bother trying to account for it. I removed it in the pushed version. > What you should be accounting for is the ensuing heap page

Re: [HACKERS] BRIN cost estimate

2017-04-06 Thread Tom Lane
David Rowley writes: > + *indexTotalCost += 0.1 * cpu_operator_cost * estimatedRanges * > + pagesPerRange; > This is trying to cost up the following code in bringetbitmap() > if (addrange) > { > BlockNumber pageno; > for (pageno = heapBlk; > pageno <= heapBlk +

Re: [HACKERS] BRIN cost estimate

2017-04-06 Thread David Rowley
On 6 April 2017 at 20:01, Emre Hasegeli wrote: >> + /* >> + * Charge a small amount per range tuple which we expect to match to. >> This >> + * is meant to reflect the costs of manipulating the bitmap. The BRIN >> scan >> + * will set a bit for each page in the range when we

Re: [HACKERS] BRIN cost estimate

2017-04-06 Thread Emre Hasegeli
> Good point. That's wrong, but I'm confused at why you kept the: > > + *indexTotalCost += selec * numTuples * cpu_index_tuple_cost; > > at all if that's the case. All the BRIN scan is going to do is build a > bitmap of the matching ranges found. My mind was not clear when I was working on it a

Re: [HACKERS] BRIN cost estimate

2017-04-05 Thread David Rowley
On 5 April 2017 at 17:34, Emre Hasegeli wrote: > >> Interested to hear comments on this. > > > I don't have chance to test it right now, but I am sure it would be an > improvement over what we have right now. There is no single correct > equation with so many unknowns we have.

Re: [HACKERS] BRIN cost estimate

2017-04-04 Thread Emre Hasegeli
> Interested to hear comments on this. I don't have chance to test it right now, but I am sure it would be an improvement over what we have right now. There is no single correct equation with so many unknowns we have. > *indexTotalCost += (numTuples * *indexSelectivity) *

Re: [HACKERS] BRIN cost estimate

2017-04-04 Thread David Rowley
On 3 April 2017 at 03:05, Emre Hasegeli wrote: > Unfortunately, I am on vacation for two weeks without my computer. I can > post another version after 18th. I know we are under time pressure for > release. I wouldn't mind if you or Alvaro would change it anyway you like.

Re: [HACKERS] BRIN cost estimate

2017-04-02 Thread Emre Hasegeli
- cond := format('%I %s %L', r.colname, r.oper, r.value); + cond := format('%s %s %L', r.colname, r.oper, r.value); Why did you change this? It seems unrelated. Must be a mistake. + indexRel = index_open(index->indexoid, AccessShareLock); + pagesPerRange = Min(BrinGetPagesPerRange(indexRel),

Re: [HACKERS] BRIN cost estimate

2017-04-02 Thread David Rowley
On 27 March 2017 at 00:44, 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

Re: [HACKERS] BRIN cost estimate

2017-03-31 Thread David Steele
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

Re: [HACKERS] BRIN cost estimate

2017-03-26 Thread Emre Hasegeli
> 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

Re: [HACKERS] BRIN cost estimate

2017-03-21 Thread Emre Hasegeli
> Not sure what you mean here. I'm not speaking of the brin index am, I > mean the get_index_stats_hook call which you've added. I see. Actually this part was from Alvaro. I haven't noticed the get_index_stats_hook call before, but it is still the same coding as btcostestimate().

Re: [HACKERS] BRIN cost estimate

2017-03-21 Thread David Rowley
On 17 March 2017 at 23:50, Emre Hasegeli wrote: >> 1. >> >> + Assert(nnumbers == 1); >> >> I think its a bad idea to Assert() this. The stat tuple can come from >> a plugin which could do anything. Seems like if we need to be certain >> of that then it should be an elog(ERROR),

Re: [HACKERS] BRIN cost estimate

2017-03-17 Thread Emre Hasegeli
> 1. > > + Assert(nnumbers == 1); > > I think its a bad idea to Assert() this. The stat tuple can come from > a plugin which could do anything. Seems like if we need to be certain > of that then it should be an elog(ERROR), maybe mention that we > expected a 1 element array, but got elements.

Re: [HACKERS] BRIN cost estimate

2017-03-13 Thread Emre Hasegeli
> Will Emre be around to make the required changes to the patch? I see > it's been a while since it was originally posted. I am around. I can post an update in a few days. Thank you for picking this up. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to

Re: [HACKERS] BRIN cost estimate

2017-03-12 Thread David Rowley
On 2 March 2017 at 04:40, Alvaro Herrera wrote: > Alvaro Herrera wrote: > >> I have added this patch to the commitfest, which I've been intending to >> get in for a long time. I'll be submitting an updated patch, if needed. > > Here is Emre's patch rebased to current

Re: [HACKERS] BRIN cost estimate

2017-03-01 Thread Alvaro Herrera
Alvaro Herrera wrote: > I have added this patch to the commitfest, which I've been intending to > get in for a long time. I'll be submitting an updated patch, if needed. Here is Emre's patch rebased to current sources. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL

Re: [HACKERS] BRIN cost estimate

2017-03-01 Thread Alvaro Herrera
Emre Hasegeli wrote: > > The patch is attached. > > Now, it is actually attached. I have added this patch to the commitfest, which I've been intending to get in for a long time. I'll be submitting an updated patch, if needed. -- Álvaro Herrerahttps://www.2ndQuadrant.com/

Re: [HACKERS] BRIN cost estimate

2015-12-24 Thread Emre Hasegeli
> The patch is attached. Now, it is actually attached. brin-correlation-v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] BRIN cost estimate

2015-12-24 Thread Emre Hasegeli
> Somebody wrote to me a few days ago that the BRIN cost estimation is > rather poor. One immediately obvious issue which I think is easily > fixed is the correlation estimate, which is currently hardcoded to 1. > > Since a BRIN scan always entails a scan of the relation in physical > order, it's

[HACKERS] BRIN cost estimate

2015-11-16 Thread Alvaro Herrera
Somebody wrote to me a few days ago that the BRIN cost estimation is rather poor. One immediately obvious issue which I think is easily fixed is the correlation estimate, which is currently hardcoded to 1. Since a BRIN scan always entails a scan of the relation in physical order, it's simple