From: | "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com> |
---|---|
To: | 'Amit Langote' <amitlangote09(at)gmail(dot)com> |
Cc: | Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>, Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, Michael Paquier <michael(at)paquier(dot)xyz>, Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, PostgreSQL-Dev <pgsql-hackers(at)postgresql(dot)org> |
Subject: | RE: [POC] Fast COPY FROM command for the table with foreign partitions |
Date: | 2020-11-26 02:42:09 |
Message-ID: | TYAPR01MB299070A2B01D63A31337F105FEF90@TYAPR01MB2990.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
From: Amit Langote <amitlangote09(at)gmail(dot)com>
> Second is whether the interface for setting ri_usesMultiInsert
> encourages situations where different modules could possibly engage in
> conflicting behaviors. I can't think of a real-life example of that
> with the current implementation, but maybe the interface provided in
> the patch makes it harder to ensure that that remains true in the
> future. Tsunakawa-san, have you encountered an example of this, maybe
> when trying to integrate this patch with some other?
Thanks. No, I pointed out purely from the standpoint of program modularity (based on structured programming?)
> Anyway, one thing we could do is rename
> ExecRelationAllowsMultiInsert() to ExecSetRelationUsesMultiInsert(),
> that is, to make it actually set ri_usesMultiInsert and have places
> like CopyFrom() call it if (and only if) its local logic allows
> multi-insert to be used. So, ri_usesMultiInsert starts out set to
> false and if a module wants to use multi-insert for a given target
> relation, it calls ExecSetRelationUsesMultiInsert() to turn the flag
> on. Also, given the confusion regarding how execPartition.c
I think separating the setting and inspection of the property into different functions will be good, at least.
> manipulates the flag, maybe change ExecFindPartition() to accept a
> Boolean parameter multi_insert, which it will pass down to
> ExecInitPartitionInfo(), which in turn will call
> ExecSetRelationUsesMultiInsert() for a given partition. Of course, if
> the logic in ExecSetRelationUsesMultiInsert() determines that
> multi-insert can't be used, for the reasons listed in the function,
> then the caller will have to live with that decision.
I can't say for sure, but it looks strange to me, because I can't find a good description of multi_insert argument for ExecFindPartition(). If we add multi_insert, I'm afraid we may want to add further arguments for other properties in the future like "Hey, get me the partition that has triggers.", "Next, pass me a partition that uses a foreign table.", etc. I think the current ExecFindPartition() is good -- "Get me a partition that accepts this row."
I wonder if ri_usesMultiInsert is really necessary. Would it cut down enough costs in the intended use case(s), say the heavyweight COPY FROM?
Regards
Takayuki Tsunakawa
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2020-11-26 02:45:04 | Re: walsender bug: stuck during shutdown |
Previous Message | Craig Ringer | 2020-11-26 02:19:00 | Re: [PATCH] LWLock self-deadlock detection |