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.
dif
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.
dif
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 use
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 no
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,
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.
>
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 that
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 / REVMAP_
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:
> > stat
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 expo
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,
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 particula
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 long-term
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. Oth
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 siz
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 IndexOpt
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
>
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, a
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 mo
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 G
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 advantag
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
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 harder,
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 use
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 no
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,
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.
>
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 that
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 / REVMAP_
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:
> > stat
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 expo
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,
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 particula
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 long-term
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. Oth
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 siz
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 i
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
38 matches
Mail list logo