Hi, Thanks for the patches. A couple mostly minor comments, to complement Melanie's review:
0001 I'm not really sure about calling this "hybrid hash-join". What does it even mean? The new comments simply describe the existing batching, and how we increment number of batches, etc. When someone says "hybrid" it usually means a combination of two very different approaches. Say, a join switching between NL and HJ would be hybrid. But this is not it. I'm not against describing the behavior, but the comment either needs to explain why it's hybrid or it should not be called that. 0002 I wouldn't call the new ExecHashJoinSaveTuple parameter spillcxt but just something generic (e.g. cxt). Yes, we're passing spillCxt, but from the function's POV it's just a pointer. I also wouldn't move the ExecHashJoinSaveTuple comment inside - it just needs to be reworded that we're expecting the context to be with the right lifespan. The function comment is the right place to document such expectations, people are less likely to read the function body. The modified comment in hashjoin.h has a some alignment issues. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
