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

2012-03-04 Thread Tom Lane
Alexander Korotkov aekorot...@gmail.com writes: On Mon, Jan 30, 2012 at 1:39 AM, Jeff Davis pg...@j-davis.com 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

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

2012-01-29 Thread Jeff Davis
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

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

2012-01-29 Thread Alexander Korotkov
On Mon, Jan 30, 2012 at 1:39 AM, Jeff Davis pg...@j-davis.com 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

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

2012-01-24 Thread Alexander Korotkov
Hi! New version of patch is attached. On Thu, Dec 22, 2011 at 11:46 AM, Jeff Davis pg...@j-davis.com 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

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

2011-12-21 Thread Jeff Davis
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)

2011-12-21 Thread Jeff Davis
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. Mapping

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

2011-12-20 Thread Alexander Korotkov
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)

2011-12-16 Thread Greg Smith
On 12/13/2011 04:04 PM, Alexander Korotkov wrote: On Mon, Dec 12, 2011 at 10:41 PM, Jeff Davis pg...@j-davis.com mailto:pg...@j-davis.com 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

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

2011-12-13 Thread Alexander Korotkov
Hi! On Mon, Dec 12, 2011 at 10:41 PM, Jeff Davis pg...@j-davis.com 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

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

2011-12-13 Thread Alexander Korotkov
On Sat, Dec 10, 2011 at 6:14 PM, Greg Smith g...@2ndquadrant.com 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

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

2011-12-12 Thread Jeff Davis
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

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

2011-12-10 Thread Greg Smith
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)

2011-12-02 Thread Alexander Korotkov
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)

2011-11-27 Thread Alexander Korotkov
On Sat, Nov 26, 2011 at 11:11 AM, Jeff Davis pg...@j-davis.com 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

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

2011-11-27 Thread Tom Lane
Alexander Korotkov aekorot...@gmail.com writes: On Sat, Nov 26, 2011 at 11:11 AM, Jeff Davis pg...@j-davis.com 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

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

2011-11-27 Thread Alexander Korotkov
On Sun, Nov 27, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us 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

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

2011-11-27 Thread Tom Lane
Alexander Korotkov aekorot...@gmail.com writes: On Sun, Nov 27, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us 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

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

2011-11-27 Thread Alexander Korotkov
On Mon, Nov 28, 2011 at 3:00 AM, Tom Lane t...@sss.pgh.pa.us 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

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

2011-11-25 Thread Jeff Davis
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)

2011-11-07 Thread Alexander Korotkov
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

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

2011-11-07 Thread Tom Lane
Alexander Korotkov aekorot...@gmail.com 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

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

2011-11-07 Thread Alexander Korotkov
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

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

2011-11-07 Thread Heikki Linnakangas
On 07.11.2011 20:36, Tom Lane wrote: Alexander Korotkovaekorot...@gmail.com 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.

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

2011-11-04 Thread Jeff Davis
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):

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

2011-11-04 Thread Jeff Davis
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 range

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

2011-11-03 Thread Heikki Linnakangas
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

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

2011-11-03 Thread David E. Wheeler
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!

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

2011-11-03 Thread Florian Pflug
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 well

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

2011-11-03 Thread Alexander Korotkov
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 as a

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

2011-11-02 Thread Heikki Linnakangas
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

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

2011-11-02 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com 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

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

2011-11-02 Thread Heikki Linnakangas
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

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

2011-11-02 Thread Heikki Linnakangas
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

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

2011-10-25 Thread Alexander Korotkov
On Mon, Oct 24, 2011 at 3:05 PM, Alexander Korotkov aekorot...@gmail.comwrote: 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

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

2011-10-24 Thread Alexander Korotkov
Hi! On Mon, Oct 17, 2011 at 12:38 PM, Jeff Davis pg...@j-davis.com 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

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

2011-10-17 Thread Jeff Davis
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. Even

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

2011-10-16 Thread Jeff Davis
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, but

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

2011-10-08 Thread Jeff Davis
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, but

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

2011-10-08 Thread Alexander Korotkov
On Sat, Oct 8, 2011 at 1:01 PM, Jeff Davis pg...@j-davis.com 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

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

2011-10-08 Thread Jeff Davis
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

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

2011-10-07 Thread Alexander Korotkov
On Fri, Oct 7, 2011 at 7:41 AM, Jeff Davis pg...@j-davis.com 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