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-21 12:39:22 |
Message-ID: | CAE2gYzyRYjrq=KU-68gE_5AEiO7zfhW-gosssLYPr71+5XXbOg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> 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(). btcostestimate() also calls get_index_stats_hook,
and then Asserts nnumbers == 1.
> hmm, before what exactly? before your patch it didn't exist. You
> introduced it into brincostestimate().
I confused by looking at my changes on my repository I made on top of
Alvaro's. I will rename it on the next version.
> 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.
Will do.
> 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 will look into it this week.
From | Date | Subject | |
---|---|---|---|
Next Message | Pavan Deolasee | 2017-03-21 12:41:21 | Re: Patch: Write Amplification Reduction Method (WARM) |
Previous Message | Haribabu Kommi | 2017-03-21 12:25:00 | Re: ANALYZE command progress checker |