Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:03 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-05-09 22:01:07 +0900, Fujii Masao wrote:
>> On Wed, May 7, 2014 at 11:55 AM, Fabrízio de Royes Mello
>> <fabriziomello(at)gmail(dot)com> wrote:
>> >
>> > On Tue, May 6, 2014 at 8:25 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
>> > wrote:
>> >>
>> >> On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres(at)2ndquadrant(dot)com>
>> >> wrote:
>> >> > Uh. They're different:
>> >> >
>> >> > Datum
>> >> > timestamp_hash(PG_FUNCTION_ARGS)
>> >> > {
>> >> > /* We can use either hashint8 or hashfloat8 directly */
>> >> > #ifdef HAVE_INT64_TIMESTAMP
>> >> > return hashint8(fcinfo);
>> >> > #else
>> >> > return hashfloat8(fcinfo);
>> >> > #endif
>> >> > }
>> >> > note it's passing fcinfo, not the datum as you do. Same with
>> >> > time_hash.. In fact your version crashes when used because it's
>> >> > dereferencing a int8 as a pointer inside hashfloat8.
>> >> Thanks, didn't notice that fcinfo was used.
>> >>
>> >
>> > Hi all,
>> >
>> > If helps, I added some regression tests to the lastest patch.
>>
>> +DATA(insert OID = 3260 ( 403 pglsn_ops PGNSP PGUID ));
>> +DATA(insert OID = 3261 ( 405 pglsn_ops PGNSP PGUID ));
>>
>> The patch looks good to me except the name of index operator class.
>
> FWIW, I've tested and looked through the patch as well.
>
>> I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
>> data type.
>
> You're right, that's marginally prettier.
>
> You plan to commit it?

Yes unless many people object the commit.

Michael,
You're now modifying the patch?

Regards,

--
Fujii Masao