Hi Robert,
On 2016/12/08 3:20, Robert Haas wrote:
> On Wed, Dec 7, 2016 at 11:53 AM, Erik Rijkers <er@xs4all.nl> wrote:
>>> My bad. The fix I sent last night for one of the cache flush issues
>>> wasn't quite right. The attached seems to fix it.
>> Yes, fixed here too. Thanks.
>
> Thanks for the report - that was a good catch.
>
> I've committed 0001 - 0006 with that correction and a few other
> adjustments. There's plenty of room for improvement here, and almost
> certainly some straight-up bugs too, but I think we're at a point
> where it will be easier and less error-prone to commit follow on
> changes incrementally rather than by continuously re-reviewing a very
> large patch set for increasingly smaller changes.
+1 and thanks a lot for your and everyone else's very patient support in
reviewing the patches.
> Some notes:
>
> * We should try to teach the executor never to scan the parent.
> That's never necessary with this system, and it might add significant
> overhead. We should also try to get rid of the idea of the parent
> having storage (i.e. a relfilenode).
Agreed, I will start investigating.
> * The fact that, in some cases, locking requirements for partitioning
> are stronger than those for inheritance is not good. We made those
> decisions for good reasons -- namely, data integrity and not crashing
> the server -- but it would certainly be good to revisit those things
> and see whether there's any room for improvement.
+1
> * I didn't commit 0007, which updates the documentation for this new
> feature. That patch removes more lines than it adds, and I suspect
> what is needed here
> is an expansion of the documentation rather than a diminishing of it.
Hmm, I had mixed feeling about what to do about that as well. So now, we
have the description of various new features buried into VI. Reference
section of the documentation, which is simply meant as a command
reference. I agree that the new partitioning warrants more expansion in
the DDL partitioning chapter. Will see how that could be done.
> * The fact that there's no implementation of row movement should be
> documented as a limitation. We should also look at removing that
> limitation.
Yes, something to improve. By the way, since we currently mention INSERT
tuple-routing directly in the description of the partitioned tables in the
CREATE TABLE command reference, is that also the place to list this
particular limitation? Or is UPDATE command reference rather the correct
place?
Thanks,
Amit