Re: [HACKERS] Add support for tuple routing to foreign partitions - Mailing list pgsql-hackers
From | Etsuro Fujita |
---|---|
Subject | Re: [HACKERS] Add support for tuple routing to foreign partitions |
Date | |
Msg-id | 5AB4EB25.4090603@lab.ntt.co.jp Whole thread Raw |
In response to | Re: [HACKERS] Add support for tuple routing to foreign partitions (Robert Haas <robertmhaas@gmail.com>) |
Responses |
Re: [HACKERS] Add support for tuple routing to foreign partitions
Re: [HACKERS] Add support for tuple routing to foreign partitions |
List | pgsql-hackers |
(2018/03/23 4:09), Robert Haas wrote: > On Tue, Feb 27, 2018 at 7:01 AM, Etsuro Fujita >> Attached is a new version of the patch set. > > I took a look at this patch set today but I really don't think we > should commit something so minimal. It's got at least four issues > that I see: > > 1. It still doesn't work for COPY, because COPY isn't going to have a > ModifyTableState. I really think it ought to be possible to come up > with an API that can handle both INSERT and COPY; I don't think it > should be necessary to have two different APIs for those two problems. > Amit managed to do it for regular tables, and I don't really see a > good reason why foreign tables need to be different. Maybe I'm missing something, but I think the proposed FDW API could be used for the COPY case as well with some modifications to core. If so, my question is: should we support COPY into foreign tables as well? I think that if we support COPY tuple routing for foreign partitions, it would be better to support direct COPY into foreign partitions as well. > 2. I previously asked why we couldn't use the existing APIs for this, > and you gave me some answer, but I can't help noticing that > postgresExecForeignRouting is an exact copy of > postgresExecForeignInsert. Apparently, some code reuse is indeed > possible here! Why not reuse the same function instead of calling a > new one? If the answer is that planSlot might be NULL in this case, > or something like that, then let's just document that. A needless > proliferation of FDW APIs is really undesirable, as it raises the bar > for every FDW author. You've got a point! I'll change the patch that way. > 3. It looks like we're just doing an INSERT for every row, which is > pretty much an anti-pattern for inserting data into a PostgreSQL > database. COPY is way faster, and even multi-row inserts are > significantly faster. I planed to work on new FDW API for using COPY for COPY tuple routing [1], but I didn't have time for that in this development cycle, so I'm re-planning to work on that for PG12. I'm not sure we can optimize that insertion using multi-row inserts because tuple routing works row by row, as you know. Anyway, I think these would be beyond the scope of the first version. > 4. It doesn't do anything about the UPDATE tuple routing problem > mentioned upthread. > > I don't necessarily think that the first patch in this area has to > solve all of those problems, and #4 in particular seems like it might > be a pretty big can of worms. OK > However, I think that getting INSERT > but not COPY, with bad performance, and with duplicated APIs, is > moving more in the wrong direction than the right one. Will fix. Thanks for reviewing the patch! Best regards, Etsuro Fujita [1] https://www.postgresql.org/message-id/23990375-45a6-5823-b0aa-a6a7a6a957f0%40lab.ntt.co.jp
pgsql-hackers by date: