Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Alexander Korotkov writes: > On Mon, Jan 30, 2012 at 1:39 AM, Jeff Davis wrote: >> Marking "ready for committer", but please apply my comment fixes at your >> discretion. > Patch with your comment fixes is attached. Applied with revisions, some cosmetic, some not so much.

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Mon, Jan 30, 2012 at 1:39 AM, Jeff Davis wrote: > Thank you for the updates. I have a small patch attached. > > The only code change I made was very minor: I changed the constants used > in the penalty function because your version used INFINITE_BOUND_PENALTY > when adding an empty range, and

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Tue, 2012-01-24 at 16:07 +0400, Alexander Korotkov wrote: > Hi! > > > New version of patch is attached. Thank you for the updates. I have a small patch attached. The only code change I made was very minor: I changed the constants used in the penalty function because your version used INFINIT

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Hi! New version of patch is attached. On Thu, Dec 22, 2011 at 11:46 AM, Jeff Davis wrote: > A few comments: > > * In range_gist_picksplit, it would be nice to have a little bit more > intuitive description of what's going on with the nonEmptyCount and > nonInfCount numbers. For instance, it appe

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Tue, 2011-12-20 at 13:22 +0400, Alexander Korotkov wrote: > Hi! > > > Studying this question little more I found that current approach of > range indexing can be dramatically inefficient in some cases. It's not > because of penalty or split implementation, but because of approach > itself. Map

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Wed, 2011-12-14 at 01:04 +0400, Alexander Korotkov wrote: > Hi! > Thank you! Attached a few changes: * Change "ordinal" to "normal" for clarity (at least to me). * Some comment cleanup * Change classes_groups to be an enum of SPLIT_LEFT and SPLIT_RIGHT, rather than using 1 and 2. * Changed the

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Hi! Studying this question little more I found that current approach of range indexing can be dramatically inefficient in some cases. It's not because of penalty or split implementation, but because of approach itself. Mapping intervals to two-dimensional space produce much better results in case

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 12/13/2011 04:04 PM, Alexander Korotkov wrote: On Mon, Dec 12, 2011 at 10:41 PM, Jeff Davis > wrote: * There's a lot of code for range_gist_penalty. Rather than having special cases for all combinations of properties in the new an original, is it poss

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sat, Dec 10, 2011 at 6:14 PM, Greg Smith wrote: > On 12/02/2011 06:48 AM, Alexander Korotkov wrote: > >> Rebased with head. >> > > Could you comment a little more on what changed? There were a couple of > areas Tom commented on: > > -General code fixes > Expensibe usage of "Max" macro is fix

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Hi! On Mon, Dec 12, 2011 at 10:41 PM, Jeff Davis wrote: > Thank you. I have attached a patch that's mostly just cleanup to this > one. > Thanks a lot for cleanup. Path with applied cleanup is attached. > Comments: > > * You use the term "ordinal range" quite a lot, which I haven't heard > befo

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Fri, 2011-12-02 at 15:48 +0400, Alexander Korotkov wrote: > Rebased with head. > Thank you. I have attached a patch that's mostly just cleanup to this one. Comments: * You use the term "ordinal range" quite a lot, which I haven't heard before. Is that a mathematical term, or do you mean somet

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 12/02/2011 06:48 AM, Alexander Korotkov wrote: Rebased with head. Could you comment a little more on what changed? There were a couple of areas Tom commented on: -General code fixes -"pull out and apply the changes related to the RANGE_CONTAIN_EMPTY flag, and also remove the <> opclass

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Rebased with head. -- With best regards, Alexander Korotkov. rangetypegist-0.4.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Mon, Nov 28, 2011 at 3:00 AM, Tom Lane wrote: > I see your point that we only need the penalty values to be comparable > for the same "new" value, but I don't think that really answers my > objection, because you've had to lobotomize the logic. As an example, > if we have a new empty range to

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Alexander Korotkov writes: > On Sun, Nov 27, 2011 at 10:43 PM, Tom Lane wrote: >> 1. penalty is using both hard-wired penalty values (1.0, 2.0, etc) and >> values obtained from subtype_diff. This is not good, because you have >> no idea what scale the subtype differences will be expressed on. T

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sun, Nov 27, 2011 at 10:43 PM, Tom Lane wrote: > > 1. penalty is using both hard-wired penalty values (1.0, 2.0, etc) and > values obtained from subtype_diff. This is not good, because you have > no idea what scale the subtype differences will be expressed on. The > hard-wired values could be

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Alexander Korotkov writes: > On Sat, Nov 26, 2011 at 11:11 AM, Jeff Davis wrote: >> There's been some significant change in rangetypes_gist.c, can you >> please rebase this patch? > OK, rebased with head. I looked at this patch a bit. I agree with the aspect of it that says "let's add a flag b

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sat, Nov 26, 2011 at 11:11 AM, Jeff Davis wrote: > > There's been some significant change in rangetypes_gist.c, can you > please rebase this patch? > OK, rebased with head. -- With best regards, Alexander Korotkov. rangetypegist-0.3.patch.gz Description: GNU Zip compressed data -- Sent

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Wed, 2011-11-09 at 20:24 +0400, Alexander Korotkov wrote: > New version of GiST for range types patch is here. This version seems > to be complete and ready for review. > There's been some significant change in rangetypes_gist.c, can you please rebase this patch? I like the patch conceptually,

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 07.11.2011 20:36, Tom Lane wrote: Alexander Korotkov writes: So, !? and ? operators are mentioned in documentation, but don't present in catalog. Are them just missed in the catalog or there is some more serious problem? IIRC, Heikki removed them from the final commit. Sounds like he miss

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

First version of GiST for range types patch is here. Comments & refactoring & testing are coming soon. -- With best regards, Alexander Korotkov. rangetypegist-0.1.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make cha

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Alexander Korotkov writes: > So, !? and ? operators are mentioned in documentation, but don't present in > catalog. Are them just missed in the catalog or there is some more serious > problem? IIRC, Heikki removed them from the final commit. Sounds like he missed some documentation.

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

During work on gist for range types I've faced with following problem: test=# select 'empty'::int4range !?; ERROR: operator does not exist: int4range !? LINE 1: select 'empty'::int4range !?; ^ HINT: No operator matches the given name and argument type(s). You mi

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Wed, 2011-11-02 at 22:59 +0200, Heikki Linnakangas wrote: > This seems to be coming from the selectivity estimation function. The > selectivity function for <@ is scalargtsel, which is usually used for > scalar > and >=. That doesn't seem right. But what do we store in the > statistics for ra

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Wed, 2011-11-02 at 21:29 +0200, Heikki Linnakangas wrote: > > + else if (lower1.infinite || upper1.infinite) > > + length1 = 1.0/0.0; > > That seems wrong. I take it that the point is to set length1 to infinity? I reworked this in commit (on my private repo, of course): 619

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Thu, Nov 3, 2011 at 3:59 PM, Heikki Linnakangas < heikki.linnakan...@enterprisedb.com> wrote: > I've committed this now, after some more cleanup. I removed the > selectivity estimation functions from operators where they were bogus, so > writing those is a clear TODO. But that can well be done

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Nov3, 2011, at 18:54 , David E. Wheeler wrote: > On Nov 3, 2011, at 4:59 AM, Heikki Linnakangas wrote: >> I've committed this now, after some more cleanup. I removed the selectivity >> estimation functions from operators where they were bogus, so writing those >> is a clear TODO. But that can

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Nov 3, 2011, at 4:59 AM, Heikki Linnakangas wrote: > I've committed this now, after some more cleanup. I removed the selectivity > estimation functions from operators where they were bogus, so writing those > is a clear TODO. But that can well be done as a separate patch. > > Thanks! Woo! C

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 03.11.2011 10:42, Jeff Davis wrote: On Wed, 2011-11-02 at 22:59 +0200, Heikki Linnakangas wrote: This seems to be coming from the selectivity estimation function. The selectivity function for<@ is scalargtsel, which is usually used for scalar> and>=. That doesn't seem right. But what do we s

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 02.11.2011 22:59, Heikki Linnakangas wrote: I'll dig deeper into this tomorrow... Forgot to mention: I have pushed what I have done this far to my git repository at git://git.postgresql.org/git/users/heikki/postgres.git, if you want to take a look. Nothing major, just garden-variety cleanu

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 01.11.2011 06:33, Jeff Davis wrote: On Mon, 2011-10-24 at 15:05 +0400, Alexander Korotkov wrote: I think implementing subtype_diff for each datatype is ok. We could implement some universal function based on minus operator and casting to double precision. But such solution might be unaccept

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Heikki Linnakangas writes: > On 01.11.2011 06:33, Jeff Davis wrote: >> + else if (lower1.infinite || upper1.infinite) >> + length1 = 1.0/0.0; > That seems wrong. I take it that the point is to set length1 to infinity? Please use get_float[48]_infinity() or get_float[48]_nan()

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On 01.11.2011 06:33, Jeff Davis wrote: On Mon, 2011-10-24 at 15:05 +0400, Alexander Korotkov wrote: I think implementing subtype_diff for each datatype is ok. We could implement some universal function based on minus operator and casting to double precision. But such solution might be unaccept

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Mon, Oct 24, 2011 at 3:05 PM, Alexander Korotkov wrote: > If we allow user to specify own gist_penalty function, then such function > should deal with: > 1) GiST-specific data structures such as GISTENTRY. > 2) Decomposing ranges using range_deserialize. > 3) Inifinities, which we could handle

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

Hi! On Mon, Oct 17, 2011 at 12:38 PM, Jeff Davis wrote: > > I started implementing subtype_diff, and I noticed that it requires > > defining an extra function for each range type. Previously, the numeric > > types could just use a cast, which was convenient for user-defined range > > types. > >

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sun, 2011-10-16 at 14:43 -0700, Jeff Davis wrote: > On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > > > The first thing caught my eye in existing GiST code is idea of > > subtype_float. float8 has limited precision and can't respresent, for > > example, varlena values good enough

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > The first thing caught my eye in existing GiST code is idea of > subtype_float. float8 has limited precision and can't respresent, for > example, varlena values good enough. Even if we have large int8 value > we can loose lower bits, b

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sat, 2011-10-08 at 18:43 +0400, Alexander Korotkov wrote: > I meant that penalty can be determined as sum of difference of old and > new bounds of range, i.e. penalty = subtype_diff_float(new_lower, > old_lower) + subtype_diff_float(old_upper, new_upper). > When we insert [100,200) into [10,+i

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Sat, Oct 8, 2011 at 1:01 PM, Jeff Davis wrote: > On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > > > The first thing caught my eye in existing GiST code is idea of > > subtype_float. float8 has limited precision and can't respresent, for > > example, varlena values good enough.

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Fri, 2011-10-07 at 12:54 +0400, Alexander Korotkov wrote: > The first thing caught my eye in existing GiST code is idea of > subtype_float. float8 has limited precision and can't respresent, for > example, varlena values good enough. Even if we have large int8 value > we can loose lower bits, b

GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

On Fri, Oct 7, 2011 at 7:41 AM, Jeff Davis wrote: > I'd prefer to include it in the initial patch. If the current GiST code > is going to be replaced, then there's not much sense reviewing/testing > it. > > You may need to consider unbounded and empty ranges specially. I made an > attempt to do s