Re: Hypothetical indexes using BRIN broken since pg10

2019-11-20 Thread Michael Paquier
On Tue, Nov 19, 2019 at 09:48:59PM +0900, Michael Paquier wrote: > Re-reading the thread. Any design change should IMO just happen on > master so as we don't take any risks with potential ABI breakages. > Even if there is not much field demand for it, that's not worth the > risk. Thinking

Re: Hypothetical indexes using BRIN broken since pg10

2019-11-19 Thread Michael Paquier
On Tue, Nov 19, 2019 at 08:37:04AM +0100, Julien Rouhaud wrote: > None from me. I'm obviously biased, but I hope that it can get > backpatched. BRIN is probably seldom used, but we shouldn't make it > harder to use it, even if it's that's only for hypothetical usage, and > even if it'll still be

Re: Hypothetical indexes using BRIN broken since pg10

2019-11-18 Thread Julien Rouhaud
On Tue, Nov 19, 2019 at 6:40 AM Michael Paquier wrote: > > On Fri, Nov 15, 2019 at 12:07:15PM +0900, Michael Paquier wrote: > > So, Heikki, are you planning to work more on that and commit a change > > close to what has been proposed upthread in [1]? It sounds to me that > > this has the

Re: Hypothetical indexes using BRIN broken since pg10

2019-11-18 Thread Michael Paquier
On Fri, Nov 15, 2019 at 12:07:15PM +0900, Michael Paquier wrote: > So, Heikki, are you planning to work more on that and commit a change > close to what has been proposed upthread in [1]? It sounds to me that > this has the advantage to be non-intrusive and a similar solution has > been used for

Re: Hypothetical indexes using BRIN broken since pg10

2019-11-14 Thread Michael Paquier
On Wed, Sep 25, 2019 at 07:03:52AM +0200, Julien Rouhaud wrote: > IIUC, if something like Heikki's patch is applied on older branch the > problem will be magically fixed from the extension point of view so > that should be safe (an extension would only need to detect the minor > version to get a

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-24 Thread Julien Rouhaud
On Tue, Sep 24, 2019 at 11:53 PM Alvaro Herrera wrote: > > I think the danger is what happens if a version of your plugin that was > compiled with the older definition runs in a Postgres which has been > recompiled with the new code. This has happened to me with previous > unnoticed ABI breaks,

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-24 Thread Alvaro Herrera
On 2019-Sep-24, Julien Rouhaud wrote: > On Mon, Sep 9, 2019 at 5:03 PM Tom Lane wrote: > > Whether we should bother back-patching a less capable stopgap fix > > is unclear to me. Yeah, it's a bug that an index adviser can't > > try a hypothetical BRIN index; but given that nobody noticed till

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-24 Thread Julien Rouhaud
On Mon, Sep 9, 2019 at 5:03 PM Tom Lane wrote: > > Alvaro Herrera from 2ndQuadrant writes: > > On 2019-Sep-02, Tom Lane wrote: > >> The right answer IMO is basically for the brinGetStats call to go > >> away from brincostestimate and instead happen during plancat.c's > >> building of the

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-09 Thread Tom Lane
Alvaro Herrera from 2ndQuadrant writes: > On 2019-Sep-02, Tom Lane wrote: >> The right answer IMO is basically for the brinGetStats call to go >> away from brincostestimate and instead happen during plancat.c's >> building of the IndexOptInfo. In the case of a hypothetical index, >> it'd fall to

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-09 Thread Alvaro Herrera from 2ndQuadrant
On 2019-Sep-02, Tom Lane wrote: > Julien Rouhaud writes: > > On Fri, Jul 26, 2019 at 1:34 PM Heikki Linnakangas wrote: > >> The patch assumes the default pages_per_range setting, but looking at > >> the code at https://github.com/HypoPG/hypopg, the extension actually > >> takes pages_per_range

Re: Hypothetical indexes using BRIN broken since pg10

2019-09-02 Thread Tom Lane
Julien Rouhaud writes: > On Fri, Jul 26, 2019 at 1:34 PM Heikki Linnakangas wrote: >> The patch assumes the default pages_per_range setting, but looking at >> the code at https://github.com/HypoPG/hypopg, the extension actually >> takes pages_per_range into account when it estimates the index

Re: Hypothetical indexes using BRIN broken since pg10

2019-07-26 Thread Julien Rouhaud
On Fri, Jul 26, 2019 at 1:34 PM Heikki Linnakangas wrote: > > There seems to be consensus on the going with the approach from the > original patch, so I had a closer look at it. > > The patch first does this: > > > > > /* > >* Obtain some data from the index itself, if possible.

Re: Hypothetical indexes using BRIN broken since pg10

2019-07-26 Thread Heikki Linnakangas
On 27/06/2019 23:09, Alvaro Herrera wrote: On 2019-Jun-27, Tom Lane wrote: Dunno, I just can't get excited about exposing REVMAP_PAGE_MAXITEMS. Especially not since we seem to agree on the long-term solution here, and what you're suggesting to Julien doesn't particularly fit into that

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Julien Rouhaud
On Thu, Jun 27, 2019 at 10:09 PM Alvaro Herrera wrote: > > On 2019-Jun-27, Tom Lane wrote: > > > Dunno, I just can't get excited about exposing REVMAP_PAGE_MAXITEMS. > > Especially not since we seem to agree on the long-term solution here, > > and what you're suggesting to Julien doesn't

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Alvaro Herrera
On 2019-Jun-27, Tom Lane wrote: > Dunno, I just can't get excited about exposing REVMAP_PAGE_MAXITEMS. > Especially not since we seem to agree on the long-term solution here, > and what you're suggesting to Julien doesn't particularly fit into > that long-term solution. Well, it was brin_page.h,

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Tom Lane
Alvaro Herrera writes: > On 2019-Jun-27, Tom Lane wrote: >> Um ... it's accounting for revmap pages already (which is why it needs >> this field set), so hasn't that ship sailed? > Yes, but does it need to know how many items there are in a revmap page? Dunno, I just can't get excited about

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Alvaro Herrera
On 2019-Jun-27, Tom Lane wrote: > Alvaro Herrera writes: > > On 2019-Jun-27, Tom Lane wrote: > >> FWIW, the proposed patch doesn't seem to me like it adds much more > >> BRIN-specific knowledge to brincostestimate than is there already. > > > It's this calculation that threw me off: > >

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Tom Lane
Alvaro Herrera writes: > On 2019-Jun-27, Tom Lane wrote: >> FWIW, the proposed patch doesn't seem to me like it adds much more >> BRIN-specific knowledge to brincostestimate than is there already. > It's this calculation that threw me off: > statsData.revmapNumPages = (indexRanges /

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Alvaro Herrera
On 2019-Jun-27, Tom Lane wrote: > FWIW, the proposed patch doesn't seem to me like it adds much more > BRIN-specific knowledge to brincostestimate than is there already. It's this calculation that threw me off: statsData.revmapNumPages = (indexRanges / REVMAP_PAGE_MAXITEMS) + 1; ISTM

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Tom Lane
Alvaro Herrera writes: > On 2019-Jun-27, Julien Rouhaud wrote: >> On Thu, Jun 27, 2019 at 8:14 PM Alvaro Herrera >> wrote: >>> I think it would look nicer to have a routine parallel to brinGetStats() >>> (brinGetStatsHypothetical?), instead of polluting selfuncs.c with these >>> gory details.

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Alvaro Herrera
On 2019-Jun-27, Julien Rouhaud wrote: > On Thu, Jun 27, 2019 at 8:14 PM Alvaro Herrera > wrote: > > I think it would look nicer to have a routine parallel to brinGetStats() > > (brinGetStatsHypothetical?), instead of polluting selfuncs.c with these > > gory details. > > I'm not opposed to it,

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Julien Rouhaud
On Thu, Jun 27, 2019 at 8:14 PM Alvaro Herrera wrote: > > Hi, thanks for the patch. Thanks for looking at it! > On 2019-Jun-27, Julien Rouhaud wrote: > > > I just realized that 7e534adcdc7 broke support for hypothetical > > indexes using BRIN am. Attached patch fix the issue. > > > > There's

Re: Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Alvaro Herrera
Hi, thanks for the patch. On 2019-Jun-27, Julien Rouhaud wrote: > I just realized that 7e534adcdc7 broke support for hypothetical > indexes using BRIN am. Attached patch fix the issue. > > There's no interface to provide the hypothetical pagesPerRange value, > so I used the default one, and

Hypothetical indexes using BRIN broken since pg10

2019-06-27 Thread Julien Rouhaud
Hello, I just realized that 7e534adcdc7 broke support for hypothetical indexes using BRIN am. Attached patch fix the issue. There's no interface to provide the hypothetical pagesPerRange value, so I used the default one, and used simple estimates. I'll add this patch to the next commitfest.