Re: [HACKERS] btree_gin and btree_gist for enums

28.02.2017 00:22, Andrew Dunstan: OK, here's the whole series of patches. Patch 1 adds the CallerFInfoFunctionCall{1,2} functions. Patch 2 adds btree_gist support for their use for non-varlena types Patch 3 does the same for varlena types (Not required for patch 4, but better to be

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/27/2017 04:41 PM, Tom Lane wrote: > Andrew Dunstan writes: >> OK, here's the whole series of patches. > I've not tested it at all, but this looks generally sane in a quick > once-over. > > A minor quibble is that in 0003, you weren't terribly consistent

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > OK, here's the whole series of patches. I've not tested it at all, but this looks generally sane in a quick once-over. A minor quibble is that in 0003, you weren't terribly consistent about argument order --- in some places you have the

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/26/2017 04:09 PM, Andrew Dunstan wrote: > > On 02/26/2017 03:26 PM, Tom Lane wrote: >> Andrew Dunstan writes: >>> This works for the btree_gin case. However, there's a difficulty for >>> btree_gist - in the puicksplit routine the cmp function is passed to

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/26/2017 03:26 PM, Tom Lane wrote: > Andrew Dunstan writes: >> This works for the btree_gin case. However, there's a difficulty for >> btree_gist - in the puicksplit routine the cmp function is passed to >> qsort() so there is no chance to pass it an flinfo

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > This works for the btree_gin case. However, there's a difficulty for > btree_gist - in the puicksplit routine the cmp function is passed to > qsort() so there is no chance to pass it an flinfo to set up the call to > the real comparison

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/25/2017 01:39 PM, Andrew Dunstan wrote: > > On 02/25/2017 01:34 PM, Tom Lane wrote: >> Andrew Dunstan writes: >>> On 02/25/2017 12:04 PM, Tom Lane wrote: I think it'd be better to leave DirectFunctionCallN alone and just invent a small number of

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/25/2017 01:34 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 02/25/2017 12:04 PM, Tom Lane wrote: >>> I think it'd be better to leave DirectFunctionCallN alone and just invent >>> a small number of CallerFInfoFunctionCallN support functions (maybe N=1

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > On 02/25/2017 12:04 PM, Tom Lane wrote: >> I think it'd be better to leave DirectFunctionCallN alone and just invent >> a small number of CallerFInfoFunctionCallN support functions (maybe N=1 >> and N=2 would be enough, at least for now).

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/25/2017 12:04 PM, Tom Lane wrote: > I think it'd be better to leave DirectFunctionCallN alone and just invent > a small number of CallerFInfoFunctionCallN support functions (maybe N=1 > and N=2 would be enough, at least for now). > > See attached. cheers andrew

Re: [HACKERS] btree_gin and btree_gist for enums

> The reason this is kind of scary is that it's just blithely assuming > that the function won't look at the *other* fields of the FmgrInfo. > If it did, it would likely get very confused, since those fields > would be describing the GIN support function, not the function we're > calling. I am

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > On 02/24/2017 05:34 PM, Andrew Dunstan wrote: >> It's occurred to me that we could reduce the code clutter in fmgr.c a >> bit by turning the DirectFunctionCall{n]Coll functions into macros >> calling these functions and passing NULL as the

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/24/2017 05:34 PM, Andrew Dunstan wrote: > > On 02/24/2017 02:55 PM, Andrew Dunstan wrote: >> On 02/24/2017 11:02 AM, Tom Lane wrote: I don't know what to call it either. In my test I used CallerContextFunctionCall2 - not sure if that's quite right, but should be close. >>>

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/24/2017 02:55 PM, Andrew Dunstan wrote: > > On 02/24/2017 11:02 AM, Tom Lane wrote: >>> I don't know what to call it either. In my test I used >>> CallerContextFunctionCall2 - not sure if that's quite right, but should >>> be close. >> CallerInfo? CallerFInfo? Or we could spell out

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/24/2017 11:02 AM, Tom Lane wrote: > Andrew Dunstan writes: >> On 02/23/2017 04:41 PM, Tom Lane wrote: >>> The reason this is kind of scary is that it's just blithely assuming >>> that the function won't look at the *other* fields of the FmgrInfo. >>> If it

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > On 02/23/2017 04:41 PM, Tom Lane wrote: >> The reason this is kind of scary is that it's just blithely assuming >> that the function won't look at the *other* fields of the FmgrInfo. >> If it did, it would likely get very confused, since

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/23/2017 04:41 PM, Tom Lane wrote: > Andrew Dunstan writes: >> I'm not entirely sure how I should replace those DirectFunctionCall2 calls. > Basically what we want is for the called function to be able to use the > fcinfo->flinfo->fn_extra and

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/23/2017 08:34 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 02/23/2017 04:41 PM, Tom Lane wrote: >>> BTW, while I'm looking at this ... isn't gin_enum_cmp broken on its face? >> Yes, that's what I'm trying to fix. > I'd forgotten where this thread

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > On 02/23/2017 04:41 PM, Tom Lane wrote: >> BTW, while I'm looking at this ... isn't gin_enum_cmp broken on its face? > Yes, that's what I'm trying to fix. I'd forgotten where this thread started. For a minute there I thought we had a

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/23/2017 04:41 PM, Tom Lane wrote: > Andrew Dunstan writes: >> I'm not entirely sure how I should replace those DirectFunctionCall2 calls. > Basically what we want is for the called function to be able to use the > fcinfo->flinfo->fn_extra and

Re: [HACKERS] btree_gin and btree_gist for enums

On 02/23/2017 04:41 PM, Tom Lane wrote: > BTW, while I'm looking at this ... isn't gin_enum_cmp broken on its face? > It's using DirectFunctionCall2 to call enum_cmp, and that's wrong because > DirectFunctionCall2 does not pass through a flinfo but enum_cmp needs to > have one. I've not tested,

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > I'm not entirely sure how I should replace those DirectFunctionCall2 calls. Basically what we want is for the called function to be able to use the fcinfo->flinfo->fn_extra and fcinfo->flinfo->fn_mcxt fields of the FmgrInfo struct that GIN

Re: [HACKERS] btree_gin and btree_gist for enums

On 11/05/2016 03:11 PM, Andrew Dunstan wrote: > > > On 11/05/2016 01:13 PM, Tom Lane wrote: >> Andrew Dunstan writes: >>> On 11/05/2016 11:46 AM, Tom Lane wrote: That may be a good fix for robustness purposes, but it seems pretty horrid from an efficiency

Re: [HACKERS] btree_gin and btree_gist for enums

On Wed, Nov 23, 2016 at 11:31 AM, Andrew Dunstan wrote: > I won't have time to fix this before the end of the Commitfest The patch is marked as "returned with feedback" in 2016-11 commitfest. Please free to submit an updated patch to the next commitfest. Regards, Hari

Re: [HACKERS] btree_gin and btree_gist for enums

I won't have time to fix this before the end of the Commitfest -- 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] btree_gin and btree_gist for enums

On 11/05/2016 01:13 PM, Tom Lane wrote: Andrew Dunstan writes: On 11/05/2016 11:46 AM, Tom Lane wrote: That may be a good fix for robustness purposes, but it seems pretty horrid from an efficiency standpoint. Where is this call, and should we be modifying it to provide

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > On 11/05/2016 11:46 AM, Tom Lane wrote: >> That may be a good fix for robustness purposes, but it seems pretty horrid >> from an efficiency standpoint. Where is this call, and should we be >> modifying it to provide a flinfo? > I thought of

Re: [HACKERS] btree_gin and btree_gist for enums

Andrew Dunstan writes: > The real problem here is that enum_cmp_internal assumes that > fcinfo->flinfo has been set up, and DirectFunctionCallN doesn't, it sets > it to NULL. > The patch below cures the problem. I'm not sure if there is a better > way. Thoughts? That may

Re: [HACKERS] btree_gin and btree_gist for enums

On 11/04/2016 04:14 PM, Emre Hasegeli wrote: The GiST part of it doesn't really work. The current patch compares oids. We need to change it to compare enumsortorder. I could make it fail easily: regression=# create type e as enum ('0', '2', '3'); CREATE TYPE regression=# alter type e add

Re: [HACKERS] btree_gin and btree_gist for enums

On 11/04/2016 04:14 PM, Emre Hasegeli wrote: Here is a patch to add enum support to btree_gin and btree_gist. I didn't include distance operations, as I didn't think it terribly important, and there isn't a simple way to compute it sanely and efficiently, so no KNN support. I don't think we

Re: [HACKERS] btree_gin and btree_gist for enums

>> Here is a patch to add enum support to btree_gin and btree_gist. I didn't >> include distance operations, as I didn't think it terribly important, and >> there isn't a simple way to compute it sanely and efficiently, so no KNN >> support. I don't think we can implement a meaningful distance

Re: [HACKERS] btree_gin and btree_gist for enums

On 03/24/2016 12:40 PM, Matt Wilmas wrote: (I notice the btree_gin docs don't mention "numeric," but it works.) Numeric does work - we have regression tests to prove it, do we should fix the docs. But I'm also curious to know why apparently we don't have distance operator support for

Re: [HACKERS] btree_gin and btree_gist for enums

On 03/24/2016 12:40 PM, Matt Wilmas wrote: It would be *really* nice to have this in 9.6. It seems it's simply filling out functionality that should already be there, right? An oversight/bug fix so it works "as advertised?" :-) I think that would be stretching the process a bit far.

Re: [HACKERS] btree_gin and btree_gist for enums

Hi Andrew, all, First message here! I didn't get around to sending an intro/"thank you all" e-mail yet, and a small performance (?) patch+idea(s)... (CPU stuff, since I don't otherwise know much about PG internals.) Anyway... - Original Message - From: "Andrew Dunstan" Sent:

Re: [HACKERS] btree_gin and btree_gist for enums

On 03/18/2016 09:54 AM, Robert Haas wrote: On Thu, Mar 17, 2016 at 11:21 AM, Andrew Dunstan wrote: On 03/17/2016 06:44 AM, Andrew Dunstan wrote: Here is a patch to add enum support to btree_gin and btree_gist. I didn't include distance operations, as I didn't think it

Re: [HACKERS] btree_gin and btree_gist for enums

On Thu, Mar 17, 2016 at 11:21 AM, Andrew Dunstan wrote: > On 03/17/2016 06:44 AM, Andrew Dunstan wrote: >> >> Here is a patch to add enum support to btree_gin and btree_gist. I didn't >> include distance operations, as I didn't think it terribly important, and >> there isn't