On Tue, Mar 11, 2014 at 5:19 AM, Peter Geoghegan <p...@heroku.com> wrote:
> On Mon, Mar 10, 2014 at 4:19 AM, Alexander Korotkov > <aekorot...@gmail.com> wrote: > > Here it is. > > So it looks like what you have here is analogous to the other problems > that I fixed with both GiST and GIN. That isn't surprising, and this > does fix my test-case. I'm not terribly happy about the lack of > explanation for the hashing in that loop, though. Why use COMP_CRC32() > at all, for one thing? > > Why do this for non-primitive jsonb hashing? > > COMP_CRC32(stack->hash_state, PATH_SEPARATOR, 1); > > Where PATH_SEPARATOR is: > > #define PATH_SEPARATOR ("\0") > > Actually, come to think of it, why not just use one hashing function > everywhere? i.e., jsonb_hash(PG_FUNCTION_ARGS)? It's already very > similar. Pretty much every hash operator support function 1 (i.e. a > particular type's hash function) is implemented with hash_any(). Can't > we just do the same here? In any case it isn't obvious why the > requirements for those two things (the hashing mechanism used by the > jsonb_hash_ops GIN opclass, and the hash operator class support > function 1 hash function) cannot be the same thing. It's because CRC32 interface allows incremental calculation while hash_any requires single chunk of memory. I don't think that unfolding everything is good idea. But we could implement incremental interface for hash_any. ------ With best regards, Alexander Korotkov.