On Fri, Apr 8, 2016 at 10:04 AM, Constantin S. Pan <kva...@gmail.com> wrote: > Here is a new version of the patch, which: > > 1. Fixes some minor stylistic issues. > > 2. Uses binaryheap (instead of a custom ugly stack) for merging.
I think we need to push this patch out to 9.7. This code has had a little review here and there from various people, but clearly not enough to push it into the tree at the very last minute. I don't think we even have enough discussion to conclude that things like the gin_shared_mem GUCs are good ideas rather than bad ones. Also, I personally find this code to be extremely low on comments. There's barely any explanation of what the overall methodology is here. Most of the functions do not have a header comment explaining what they do. The code hasn't been run through pgindent. There's no check to see whether the computation that will be done inside the GIN index is parallel-safe; what if it's an expression index on an unsafe function? Opening the heap and index with no lock in the worker is wrong; the worker should use the same lock mode as the leader. That's just on a quick read-through; I'm sure there's more. I'm going to move this to the next CommitFest. Hopefully someone will volunteer to do some serious review of this, because the feature sounds cool. Possibly that person will even be me. But it will not be today. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
