On Tue, Aug 15, 2017 at 10:15 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Aug 10, 2017 at 8:00 AM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>> Attached patches with the comments addressed.
>
> I have committed 0001-0003 as 480f1f4329f1bf8bfbbcda8ed233851e1b110ad4
> and e139f1953f29db245f60a7acb72fccb1e05d2442.
Thanks a lot Robert. Some less patches to maintain :).
>
> 0004 doesn't apply any more, probably due to commit
> d57929afc7063431f80a0ac4c510fc39aacd22e6. I think something along
> these lines could be separately committed prior to the main patch, and
> I think that would be a good idea just to flush out any bugs in this
> part independently of the rest. However, I also think that we
> probably ought to try to get Amit Langote's changes to this function
> to repair the locking order and expand in bound order committed before
> proceeding with these changes.
I am reviewing those changes and contribute to that thread if necessary.
>
> In fact, I think there's a certain amount of conflict between what's
> being discussed over there and what you're trying to do here. In that
> thread, we propose to move partitioned tables at any level to the
> front of the inheritance expansion. Here, however, you want to expand
> level by level. I think partitioned-tables-first is the right
> approach for the reasons discussed on the other thread; namely, we
> want to be able to prune leaf partitions before expanding them, but
> that requires us to expand all the non-leaf tables first to maintain a
> consistent locking order in all scenarios. So the approach you've
> taken in this patch may need to be re-thought somewhat.
>
There are two ways we can do this
1. In expand_inherited_rtentry(), remember (childRTE and childRTIndex)
or just childRTIndex (using this we can fetch childRTE calling
rtfetch()) of intermediate partitioned tables. Once we are done
expanding immediate children, call expand_inherited_rtentry()
recursively on this list.
2. expand_inherited_tables() scans root->parse->rtable only upto the
end of original range table list. Make it go beyond that end,
expanding any new entries added for intermediate partitions.
FWIW, the first option allows us to keep all AppendRelInfos
corresponding to one partitioned relation together and also expands
the whole partition hierarchy in one go. Second will require minimal
changes to expand_inherited_rtentry(). Both approaches will spend time
scanning same number of RTE; the first will have them in different
lists, and second will have them in root->parse->rtable. I don't see
one being more attractive than the other. Do you have any opinion?
I will submit the rebased patches after reviewing/adjusting Amit's
changes and also the changes in expand_inherited_rtentry() after we
have concluded the approach to be taken.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company