From: Amit Langote <amitlangote09@gmail.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
thefuture 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
theheavyweight COPY FROM?
Regards
Takayuki Tsunakawa
