RE: [POC] Fast COPY FROM command for the table with foreign partitions

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

In response to

Responses

Browse pgsql-hackers by date

  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