Re: [HACKERS] BRIN range operator class

2015-05-15 Thread Alvaro Herrera
Emre Hasegeli wrote: I pushed patches 04 and 07, as well as adopting some of the changes to the regression test in 06. I'm afraid I caused a bit of merge pain for you -- sorry about that. No problem. I rebased the remaining ones. Thanks, pushed. There was a proposed change by Emre to

Re: [HACKERS] BRIN range operator class

2015-05-13 Thread Alvaro Herrera
Emre Hasegeli wrote: I pushed patches 04 and 07, as well as adopting some of the changes to the regression test in 06. I'm afraid I caused a bit of merge pain for you -- sorry about that. No problem. I rebased the remaining ones. Thanks! After some back-and-forth between Emre and me,

Re: [HACKERS] BRIN range operator class

2015-05-12 Thread Alvaro Herrera
Alvaro Herrera wrote: In patch 05, you use straight etc comparisons of point/box values. All the other code in that file AFAICS uses FPlt() macros and others; I assume we should do likewise. Oooh, looking at the history of this I just realized that the comments signed tgl are actually Thomas

Re: [HACKERS] BRIN range operator class

2015-05-12 Thread Alvaro Herrera
Emre Hasegeli wrote: I pushed patches 04 and 07, as well as adopting some of the changes to the regression test in 06. I'm afraid I caused a bit of merge pain for you -- sorry about that. No problem. I rebased the remaining ones. In patch 05, you use straight etc comparisons of

Re: [HACKERS] BRIN range operator class

2015-05-12 Thread Alvaro Herrera
So, in reading these patches, it came to me that we might want to have pg_upgrade mark indexes invalid if we in the future change the implementation of some opclass. For instance, the inclusion opclass submitted here uses three columns: the indexed value itself, plus two booleans; each of these

Re: [HACKERS] BRIN range operator class

2015-05-12 Thread Heikki Linnakangas
On 05/12/2015 10:49 PM, Alvaro Herrera wrote: If in the future, for instance, we come up with a way to store the ipv4 plus ipv6 info, we will want to change the page format. If we add a page version to the metapage, we can detect the change at pg_upgrade time and force a reindex of the index.

Re: [HACKERS] BRIN range operator class

2015-05-12 Thread Alvaro Herrera
Heikki Linnakangas wrote: On 05/12/2015 10:49 PM, Alvaro Herrera wrote: If in the future, for instance, we come up with a way to store the ipv4 plus ipv6 info, we will want to change the page format. If we add a page version to the metapage, we can detect the change at pg_upgrade time and

Re: [HACKERS] BRIN range operator class

2015-05-10 Thread Emre Hasegeli
I pushed patches 04 and 07, as well as adopting some of the changes to the regression test in 06. I'm afraid I caused a bit of merge pain for you -- sorry about that. No problem. I rebased the remaining ones. brin-inclusion-v09-02-strategy-numbers.patch Description: Binary data

Re: [HACKERS] BRIN range operator class

2015-05-07 Thread Alvaro Herrera
Emre Hasegeli wrote: After looking at 05 again, I don't like the same as % business. Creating a whole new class of exceptions is not my thing, particularly not in a regression test whose sole purpose is to look for exceptional (a.k.a. wrong) cases. I would much rather define the opclasses

Re: [HACKERS] BRIN range operator class

2015-05-06 Thread Emre Hasegeli
Looking at patch 04, it seems to me that it would be better to have the OpcInfo struct carry the typecache struct rather than the type OID, so that we can avoid repeated typecache lookups in brin_deform_tuple; Here's the patch. Looks better to me. I will incorporate with this patch. --

Re: [HACKERS] BRIN range operator class

2015-05-06 Thread Emre Hasegeli
Can you please explain what is the purpose of patch 07? I'm not sure I understand; are we trying to avoid having to add pg_amproc entries for these operators and instead piggy-back on btree opclass definitions? Not too much in love with that idea; I see that there is less tedium in that the

Re: [HACKERS] BRIN range operator class

2015-05-06 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes: Let's think together and try to find a reasonable way to get the union procedures tested regularly. It is pretty clear that having them run only when the race condition occurs is not acceptable; bugs go unnoticed. [ just a drive-by comment... ]

Re: [HACKERS] BRIN range operator class

2015-05-06 Thread Alvaro Herrera
I again have to refuse the notion that removing the assert-only block without any replacement is acceptable. I just spent a lot of time tracking down what turned out to be a bug in your patch 07: /* Adjust maximum, if B's max is greater than A's max */ - needsadj =

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Andreas Karlsson
On 05/05/2015 01:10 PM, Emre Hasegeli wrote: I already replied his email [1]. Which issues do you mean? Sorry, my bad please ignore the previous email. -- Andreas Karlsson -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Alvaro Herrera
Looking at patch 04, it seems to me that it would be better to have the OpcInfo struct carry the typecache struct rather than the type OID, so that we can avoid repeated typecache lookups in brin_deform_tuple; something like /* struct returned by OpcInfo amproc */ typedef struct BrinOpcInfo {

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Alvaro Herrera
Alvaro Herrera wrote: Looking at patch 04, it seems to me that it would be better to have the OpcInfo struct carry the typecache struct rather than the type OID, so that we can avoid repeated typecache lookups in brin_deform_tuple; Here's the patch. -- Álvaro Herrera

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Alvaro Herrera
After looking at 05 again, I don't like the same as % business. Creating a whole new class of exceptions is not my thing, particularly not in a regression test whose sole purpose is to look for exceptional (a.k.a. wrong) cases. I would much rather define the opclasses for those two datatypes

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Andreas Karlsson
On 05/05/2015 11:57 AM, Emre Hasegeli wrote: From my point of view as a reviewer this patch set is very close to being committable. Thank you. The new versions are attached. Nice, I think it is ready now other than the issues Alvaro raised in his review[1]. Have you given those any

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Andreas Karlsson
On 05/05/2015 04:24 AM, Alvaro Herrera wrote: Stefan Keller wrote: I'm keen to see if a PostGIS specialist jumps in and adds PostGIS geometry support. Did you test the patch proposed here already? It could be a very good contribution. Indeed, I have done some testing of the patch but more

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Emre Hasegeli
Indeed, I have done some testing of the patch but more people testing would be nice. The inclusion opclass should work for other data types as long required operators and SQL level support functions are supplied. Maybe it would work for PostGIS, too. -- Sent via pgsql-hackers mailing list

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Alvaro Herrera
Can you please explain what is the purpose of patch 07? I'm not sure I understand; are we trying to avoid having to add pg_amproc entries for these operators and instead piggy-back on btree opclass definitions? Not too much in love with that idea; I see that there is less tedium in that the brin

Re: [HACKERS] BRIN range operator class

2015-05-05 Thread Emre Hasegeli
Nice, I think it is ready now other than the issues Alvaro raised in his review[1]. Have you given those any thought? I already replied his email [1]. Which issues do you mean? [1] http://www.postgresql.org/message-id/CAE2gYzxQ-Gk3q3jYWT=1enlebsgcgu28+1axml4omcwjbkp...@mail.gmail.com --

Re: [HACKERS] BRIN range operator class

2015-05-04 Thread Stefan Keller
Hi, 2015-05-05 2:51 GMT+02:00 Andreas Karlsson andr...@proxel.se: From my point of view as a reviewer this patch set is very close to being committable. I'd like to thank already now to all committers and reviewers and hope BRIN makes it into PG 9.5. As a database instructor, conference

Re: [HACKERS] BRIN range operator class

2015-05-04 Thread Andreas Karlsson
From my point of view as a reviewer this patch set is very close to being committable. = brin-inclusion-v06-01-sql-level-support-functions.patch This patch looks good. = brin-inclusion-v06-02-strategy-numbers.patch This patch looks good, but shouldn't it be merged with 07? =

Re: [HACKERS] BRIN range operator class

2015-05-04 Thread Alvaro Herrera
Stefan Keller wrote: Hi, 2015-05-05 2:51 GMT+02:00 Andreas Karlsson andr...@proxel.se: From my point of view as a reviewer this patch set is very close to being committable. I'd like to thank already now to all committers and reviewers and hope BRIN makes it into PG 9.5. As a database

Re: [HACKERS] BRIN range operator class

2015-05-02 Thread Andreas Karlsson
On 04/06/2015 09:36 PM, Emre Hasegeli wrote: Yes but they were also required by this patch. This version adds more functions and operators. I can split them appropriately after your review. Ok, sounds fine to me. It is now split. In which order should I apply the patches? I also agree

Re: [HACKERS] BRIN range operator class

2015-04-30 Thread Robert Haas
On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thanks for the updated patch; I will at it as soon as time allows. (Not really all that soon, regrettably.) Judging from a quick look, I think patches 1 and 5 can be committed quickly; they imply no changes to

Re: [HACKERS] BRIN range operator class

2015-04-30 Thread Alvaro Herrera
Robert Haas wrote: On Mon, Apr 6, 2015 at 5:17 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thanks for the updated patch; I will at it as soon as time allows. (Not really all that soon, regrettably.) Judging from a quick look, I think patches 1 and 5 can be committed quickly;

Re: [HACKERS] BRIN range operator class

2015-04-14 Thread Emre Hasegeli
Judging from a quick look, I think patches 1 and 5 can be committed quickly; they imply no changes to other parts of BRIN. (Not sure why 1 and 5 are separate. Any reason for this?) Also patch 2. Not much reason except that 1 includes only functions, but 5 includes operators. Patch 4 looks

Re: [HACKERS] BRIN range operator class

2015-04-06 Thread Alvaro Herrera
Thanks for the updated patch; I will at it as soon as time allows. (Not really all that soon, regrettably.) Judging from a quick look, I think patches 1 and 5 can be committed quickly; they imply no changes to other parts of BRIN. (Not sure why 1 and 5 are separate. Any reason for this?) Also

Re: [HACKERS] BRIN range operator class

2015-03-07 Thread Andreas Karlsson
On 02/11/2015 07:34 PM, Emre Hasegeli wrote: The current code compiles but the brin test suite fails. Now, only a test in . Yeah, there is still a test which fails in opr_sanity. Yes but they were also required by this patch. This version adds more functions and operators. I can split

Re: [HACKERS] BRIN range operator class

2015-02-13 Thread Michael Paquier
On Thu, Feb 12, 2015 at 3:34 AM, Emre Hasegeli e...@hasegeli.com wrote: Thank you for looking at my patch again. New version is attached with a lot of changes and point data type support. Patch is moved to next CF 2015-02 as work is still going on. -- Michael

Re: [HACKERS] BRIN range operator class

2015-01-22 Thread Alvaro Herrera
Can you please break up this patch? I think I see three patches, 1. add sql-callable functions such as inet_merge, network_overright, etc etc. These need documentation and a trivial regression test somewhere. 2. necessary changes to header files (skey.h etc) 3. the inclusion opclass itself

Re: [HACKERS] BRIN range operator class

2015-01-10 Thread Andreas Karlsson
Hi, I made a quick review for your patch, but I would like to see someone who was involved in the BRIN work comment on Emre's design issues. I will try to answer them as best as I can below. I think minimax indexes on range types seems very useful, and inet/cidr too. I have no idea about

Re: [HACKERS] BRIN range operator class

2014-12-14 Thread Emre Hasegeli
I thought we can do better than minmax for the inet data type, and ended up with a generalized opclass supporting both inet and range types. Patch based on minmax-v20 attached. It works well except a few small problems. I will improve the patch and add into a commitfest after BRIN