From: Amit Langote <amitlangot...@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 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
