Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-20 Thread Robert Haas
On Sun, Oct 18, 2015 at 2:52 PM, Peter Geoghegan wrote: > On Thu, Oct 15, 2015 at 9:07 AM, Robert Haas wrote: >> Would you be willing to try coding it up that way so we can see how it looks? > > I attach a patch that does it that way. That looks good to

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-18 Thread Peter Geoghegan
On Thu, Oct 15, 2015 at 9:07 AM, Robert Haas wrote: > Would you be willing to try coding it up that way so we can see how it looks? I attach a patch that does it that way. Note that I will be away for until late this month. Do not expect a response from me before then,

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-15 Thread Robert Haas
On Wed, Oct 14, 2015 at 7:11 PM, Peter Geoghegan wrote: > On Wed, Oct 14, 2015 at 4:09 PM, Robert Haas wrote: >> I wonder if it wouldn't be better to just add a separate Boolean >> indicating exactly the thing we care about. This doesn't seem >>

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-14 Thread Peter Geoghegan
On Wed, Oct 14, 2015 at 4:09 PM, Robert Haas wrote: > I wonder if it wouldn't be better to just add a separate Boolean > indicating exactly the thing we care about. This doesn't seem > particularly easy to understand and verify. I'm not really sure that that's an

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-14 Thread Robert Haas
On Wed, Oct 14, 2015 at 7:05 PM, Peter Geoghegan wrote: > On Mon, Oct 12, 2015 at 12:31 PM, Peter Geoghegan wrote: >> I'll consider a more comprehensive fix. > > I attach a revised fix that considers the problem of misinterpreting > the contents of the buffers

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-14 Thread Peter Geoghegan
On Mon, Oct 12, 2015 at 12:31 PM, Peter Geoghegan wrote: > I'll consider a more comprehensive fix. I attach a revised fix that considers the problem of misinterpreting the contents of the buffers in both directions. Thanks -- Peter Geoghegan From

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-12 Thread Peter Geoghegan
On Sat, Oct 10, 2015 at 12:32 PM, Peter Geoghegan wrote: > I noticed that there is still one comment that I really should have > removed as part of this work. I also noticed that I failed to reset the last_returned strcoll() cache variable as part of an abbreviation call,

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-12 Thread Peter Geoghegan
On Mon, Oct 12, 2015 at 12:47 AM, Peter Geoghegan wrote: > I also noticed that I failed to reset the last_returned strcoll() > cache variable as part of an abbreviation call, despite the fact that > tapesort may freely interleave conversions with comparisons, while > reusing buf1

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-12 Thread Peter Geoghegan
On Mon, Oct 12, 2015 at 4:15 PM, Robert Haas wrote: > In this case, I think > the best thing for me to do right now is wait to commit anything > further until you have had a chance to go over this and come up with a > fix or set of fixes that you think are completely, 100%

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-12 Thread Robert Haas
On Mon, Oct 12, 2015 at 3:31 PM, Peter Geoghegan wrote: > On Mon, Oct 12, 2015 at 12:47 AM, Peter Geoghegan wrote: >> I also noticed that I failed to reset the last_returned strcoll() >> cache variable as part of an abbreviation call, despite the fact that >>

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-10 Thread Peter Geoghegan
On Fri, Oct 9, 2015 at 5:54 PM, Robert Haas wrote: > I think that is true. I spent some time thinking about whether the > way you used INT_MIN as a sentinel value should be changed around > somehow, but ultimately I decided that it wasn't too bad and that > suggesting

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Robert Haas
On Fri, Oct 9, 2015 at 3:33 PM, Peter Geoghegan wrote: > On Fri, Oct 9, 2015 at 12:11 PM, Robert Haas wrote: >> OK, committed that way. > > Thank you. You're welcome. After some study and experimentation, I've committed the other part also. -- Robert

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Peter Geoghegan
On Fri, Oct 9, 2015 at 4:39 PM, Robert Haas wrote: > You're welcome. After some study and experimentation, I've committed > the other part also. Fantastic. I guess the precedent of the 9.5 text equality fast path made this discussion way smoother than last time, since

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Robert Haas
On Fri, Oct 9, 2015 at 7:49 PM, Peter Geoghegan wrote: > On Fri, Oct 9, 2015 at 4:39 PM, Robert Haas wrote: >> You're welcome. After some study and experimentation, I've committed >> the other part also. > > Fantastic. I guess the precedent of the 9.5

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Peter Geoghegan
On Fri, Oct 9, 2015 at 12:11 PM, Robert Haas wrote: > OK, committed that way. Just for the record, with the same "cities" table as my original post to this thread, this query: select count(distinct(city)) from cities; Goes from taking about 296ms (once it stabilizes), to

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Robert Haas
On Thu, Oct 8, 2015 at 8:20 PM, Peter Geoghegan wrote: > I should point out that I did not call the macro DatumToBigEndian(), > because it's actually the other way around. I called it > DatumToLittleEndian(), since the unsigned integer comparator would > work correctly on

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Peter Geoghegan
On Fri, Oct 9, 2015 at 11:44 AM, Robert Haas wrote: > Hmm. But then this doesn't seem to make much sense: > > + * Rearrange the bytes of a Datum into little-endian order from big-endian > + * order. On big-endian machines, this does nothing at all. > > Rearranging bytes

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Peter Geoghegan
On Fri, Oct 9, 2015 at 12:11 PM, Robert Haas wrote: > OK, committed that way. Thank you. -- Peter Geoghegan -- 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] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-09 Thread Robert Haas
On Fri, Oct 9, 2015 at 2:48 PM, Peter Geoghegan wrote: > On Fri, Oct 9, 2015 at 11:44 AM, Robert Haas wrote: >> Hmm. But then this doesn't seem to make much sense: >> >> + * Rearrange the bytes of a Datum into little-endian order from big-endian >> + *

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-08 Thread Robert Haas
On Wed, Oct 7, 2015 at 8:09 PM, Peter Geoghegan wrote: > On Tue, Oct 6, 2015 at 1:16 PM, Robert Haas wrote: >> If you would care to revise the patch accordingly, I will commit it >> (barring objections from others, of course). > > Here is a revision of

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-08 Thread Peter Geoghegan
On Thu, Oct 8, 2015 at 10:13 AM, Robert Haas wrote: > It seems to me that (1) ABBREV_STRING_UINT isn't > a great name for this and (2) the comment is awfully long for the > thing to which it refers. I suggest that we instead call it > DatumToBigEndian(), put it pg_bswap.h,

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-08 Thread Peter Geoghegan
On Thu, Oct 8, 2015 at 11:37 AM, Robert Haas wrote: > I'm not convinced. Doesn't this exact same concept get used for > over-the-wire communication between BE and LE machines? There, this > operation is spelled htonl/ntohl. Some systems even have htonll, but > I'm sure

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-08 Thread Robert Haas
On Thu, Oct 8, 2015 at 2:07 PM, Peter Geoghegan wrote: > On Thu, Oct 8, 2015 at 10:13 AM, Robert Haas wrote: >> It seems to me that (1) ABBREV_STRING_UINT isn't >> a great name for this and (2) the comment is awfully long for the >> thing to which it

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-07 Thread Peter Geoghegan
On Tue, Oct 6, 2015 at 1:16 PM, Robert Haas wrote: > If you would care to revise the patch accordingly, I will commit it > (barring objections from others, of course). Here is a revision of 0001-*, with both BSWAP32() and BSWAP64() in a new header, src/port/pg_bswap.h. No

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-06 Thread Peter Geoghegan
On Tue, Oct 6, 2015 at 1:07 PM, Robert Haas wrote: > Reviewing 0001, I'm happy to see us add bswap64, but I'm not sure we > should put it in c.h, because that's included by absolutely > everything. How about putting it in a new #include inside src/port, > like

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-06 Thread Robert Haas
On Sun, Oct 4, 2015 at 2:17 AM, Peter Geoghegan wrote: > On Tue, Aug 4, 2015 at 12:41 PM, Robert Haas wrote: >> Some comments: > > I attach a new version of the patch series that incorporates all your > feedback. The patch series is now made cumulative in

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-06 Thread Peter Geoghegan
On Tue, Oct 6, 2015 at 1:16 PM, Robert Haas wrote: >> I guess I imagined that bswap64() was fundamental infrastructure, but >> on second thought that's not actually in evidence -- it is not already >> needed in plenty of other places. So yeah, works for me. > > If you would

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-06 Thread Robert Haas
On Tue, Oct 6, 2015 at 4:09 PM, Peter Geoghegan wrote: > On Tue, Oct 6, 2015 at 1:07 PM, Robert Haas wrote: >> Reviewing 0001, I'm happy to see us add bswap64, but I'm not sure we >> should put it in c.h, because that's included by absolutely >>

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-06 Thread Robert Haas
On Tue, Oct 6, 2015 at 4:26 PM, Peter Geoghegan wrote: > On Tue, Oct 6, 2015 at 1:16 PM, Robert Haas wrote: >>> I guess I imagined that bswap64() was fundamental infrastructure, but >>> on second thought that's not actually in evidence -- it is not already

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-10-04 Thread Peter Geoghegan
On Tue, Aug 4, 2015 at 12:41 PM, Robert Haas wrote: > Some comments: I attach a new version of the patch series that incorporates all your feedback. The patch series is now made cumulative in a way that makes it easy for someone to independently commit the unsigned integer

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-08-04 Thread Robert Haas
On Fri, Jul 3, 2015 at 8:33 PM, Peter Geoghegan p...@heroku.com wrote: Since apparently we're back to development work, I thought it was time to share a patch implementing a few additional simple tricks to make sorting text under a non-C locale even faster than in 9.5. These techniques are

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-08-04 Thread Peter Geoghegan
On Tue, Aug 4, 2015 at 12:41 PM, Robert Haas robertmh...@gmail.com wrote: Interesting work. Thanks. 1. My biggest gripe with this patch is that the comments are not easy to understand. Of course everybody may prefer something different here; I'm just telling you what I think. I have

Re: [HACKERS] More work on SortSupport for text - strcoll() and strxfrm() caching

2015-08-04 Thread Peter Geoghegan
On Tue, Aug 4, 2015 at 1:30 PM, Peter Geoghegan p...@heroku.com wrote: 2. I believe the change to bttextcmp_abbrev() should be pulled out into a separate patch and committed separately. That part seems like a slam dunk. Makes sense. BTW, I want to put the string_uint() macro in a common