On Thu, Apr 1, 2021 at 12:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Langote <amitlangote09@gmail.com> writes:
> > On Tue, Mar 30, 2021 at 1:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Here's a v13 patchset that I feel pretty good about.
>
> > Thanks. After staring at this for a day now, I do too.
>
> Thanks for looking! Pushed after some more docs-fiddling and a final
> read-through. I think the only code change from v13 is that I decided
> to convert ExecGetJunkAttribute into a "static inline", since it's
> just a thin wrapper around slot_getattr(). Doesn't really help
> performance much, but it shouldn't hurt.
Thanks a lot.
> > ... The drop as partition count increases can
> > be attributed to the fact that with a generic plan, there are a bunch
> > of steps that must be done across all partitions, such as
> > AcauireExecutorLocks(), ExecCheckRTPerms(), per-result-rel
> > initializations in ExecInitModifyTable(), etc., even with the patched.
> > As mentioned upthread, [1] can help with the last bit.
>
> I'll try to find some time to look at that one.
>
> I'd previously been thinking that we couldn't be lazy about applying
> most of those steps at executor startup, but on second thought,
> ExecCheckRTPerms should be a no-op anyway for child tables.
Yeah, David did say that in that thread:
https://www.postgresql.org/message-id/CAApHDvqPzsMcKLRpmNpUW97PmaQDTmD7b2BayEPS5AN4LY-0bA%40mail.gmail.com
> So
> maybe it would be okay to not take a lock, much less do the other
> stuff, until the particular child table is stored into.
Note that the patch over there doesn't do anything about
AcquireExecutorLocks() bottleneck, as there are some yet-unsolved race
conditions that were previously discussed here:
https://www.postgresql.org/message-id/flat/CAKJS1f_kfRQ3ZpjQyHC7=PK9vrhxiHBQFZ+hc0JCwwnRKkF3hg@mail.gmail.com
Anyway, I'll post the rebased version of the patch that we do have.
--
Amit Langote
EDB: http://www.enterprisedb.com