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
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 +
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
> 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
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.
> 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) *
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.
- 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),
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
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
> 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
> 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().
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),
> 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.
> 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
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
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
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/
> 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
> 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
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
21 matches
Mail list logo