Re: expanding inheritance in partition bound order
Amit Langote <langote_amit_f8@lab.ntt.co.jp>
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Robert Haas <robertmhaas@gmail.com>,
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Amit Khandekar <amitdkhan.pg@gmail.com>,
Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-08-18T05:25:10Z
Lists: pgsql-hackers
Attachments
- 0001-Decouple-RelationGetPartitionDispatchInfo-from-execu.patch (text/plain) patch 0001
- 0002-Teach-expand_inherited_rtentry-to-use-partition-boun.patch (text/plain) patch 0002
On 2017/08/18 4:54, Robert Haas wrote: > On Thu, Aug 17, 2017 at 8:39 AM, Ashutosh Bapat > <ashutosh.bapat@enterprisedb.com> wrote: >> [2] had a patch with some changes to the original patch you posted. I >> didn't describe those changes in my mail, since they rearranged the >> comments. Those changes are not part of this patch and you haven't >> comments about those changes as well. If you have intentionally >> excluded those changes, it's fine. In case, you haven't reviewed them, >> please see if they are good to be incorporated. > > I took a quick look at your version but I think I like Amit's fine the > way it is, so committed that and back-patched it to v10. Thanks for committing. > I find 0002 pretty ugly as things stand. We get a bunch of tuple maps > that we don't really need, only to turn around and free them. We get > a bunch of tuple slots that we don't need, only to turn around and > drop them. We don't really need the PartitionDispatch objects either, > except for the OIDs they contain. There's a lot of extra stuff being > computed here that is really irrelevant for this purpose. I think we > should try to clean that up somehow. One way to do that might be to reverse the order of the remaining patches and put the patch to refactor RelationGetPartitionDispatchInfo() first. With that refactoring, PartitionDispatch itself has become much simpler in that it does not contain a relcache reference to be closed eventually by the caller, the tuple map, and the tuple table slot. Since those things are required for tuple-routing, the refactoring makes ExecSetupPartitionTupleRouting itself create them from the (minimal) information returned by RelationGetPartitionDispatchInfo and ultimately destroy when done using it. I kept the indexes field in PartitionDispatchData though, because it's essentially free to create while we are walking the partition tree in RelationGetPartitionDispatchInfo() and it seems undesirable to make the caller compute that information (indexes) by traversing the partition tree all over again, if it doesn't otherwise have to. I am still considering some counter-arguments raised by Amit Khandekar about this last assertion. Thoughts? Thanks, Amit
Commits
-
Make RelationGetPartitionDispatchInfo expand depth-first.
- 77b6b5e9ceca 11.0 landed
-
Expand partitioned tables in PartDesc order.
- 30833ba154e0 11.0 landed
-
Don't lock tables in RelationGetPartitionDispatchInfo.
- 7c0ca2900f7c 10.0 landed
- 54cde0c4c058 11.0 landed
-
Speed up dropping tables with many partitions.
- c1e0e7e1d790 10.0 cited