> So, overall, we feel that the code from lines 2689 until 2691 and from 2740 to 2766 could use further clarification with regard to switching from parent to child partition and sibling to sibling partition as well as regarding saving relinfo for partitions that have not been seen before in the proute->partitions[]
I hope it's more clear now that I've got rid of saved_resultRelInfo.
I think all of the refactoring and clarifications look good to me and are much more clear.
One small additional typo I noticed:
In the patched code on line 2555, there appears to be a typo: /* ... * inserting into and act differently if the tuples that have already * been processed any prepared for insertion are not there.
*/
The word "any" here is probably wrong, though I am actually not sure what was meant.
Though it is from the existing comments, it would be nice to spell out once what is meant by "BR trigger". I'm sure it is mentioned elsewhere in the code, but, since it is not easily googled, it might be helpful to specify.
Many thanks to both of you for the thorough review. I hope the attached addresses all the concerns.
I feel that the code itself is clear and feel our concerns are addressed.
One final note: I'm not entirely convinced we need this adaptive code, but it seems easy enough to rip it back out if it's more trouble than it's worth. But if the other option is a GUC, then I'd rather stick with the adaptive code, it's likely going to do much better than a GUC since it can change itself during the copy which will be useful when the stream contains a mix small and large sets of consecutive tuples which belong to the same partition.
Though the v2 numbers do look better, it doesn't complete alleviate the slow-down in the worst case. Perhaps the GUC and the adaptive code are not alternatives and could instead be used together. You could even make the actual RECHECK_MULTI_INSERT_THRESHOLD the GUC.
However, I think that the decision as to whether or not it makes sense to do the adaptive code without a GUC is really based on the average use case, to which I can't really speak.
The counter-argument I see, however, is that it is not acceptable to have completely un-optimized insertion of data into partition tables and that an overwhelming flood of GUCs is undesirable.
