On 7/18/13 7:57 PM, Karol Trzcionka wrote:
> Current head 4cbe3ac3e86790d05c569de4585e5075a62a9b41 -> patch applies
> correct (only change needed in parallel_schedule).
> However it fails on own regression tests (other tests pass).
I got a rejected hunk in src/backend/nodes/nodeFuncs.c as well as that
parallel_schedule issue. Maybe you didn't get the nodeFuncs change but
didn't notice that? That might explain why the tests didn't work for
you either.
Attached is an updated patch where I tried to only fix the two small
hunks of bit rot. I get "All 140 tests passed" here, on a Mac no less.
I did a brief code scan through the patch just to get a feel for how the
feature is put together, and what you'd need to know for a deeper
review. (I'm trying to get customer time approved to work on this a lot
more) The code was easier to follow than I expected. The way it
completely avoids even getting into the security label integration yet
seems like a successful design partitioning. This isn't nearly as scary
as the SEPostgres patches. There are some useful looking utility
functions that dump information about what's going on too.
The bulk of the complexity is how the feature modifies query nodes to
restrict what rows come through them. Some familiarity with that part
of the code is what you'd need to take on reviewing this in detail.
That and a week of time to spend trudging through it. If anyone is
looking for an educational challenge on query execution, marching
through all of these changes to validate they work as expected would do
that.
--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com