Hi Georgios, On Tue, Mar 16, 2021 at 5:12 PM <gkokola...@pm.me> wrote: > On Tuesday, March 16, 2021 6:13 AM, Amit Langote <amitlangot...@gmail.com> > wrote: > > On Fri, Mar 12, 2021 at 7:59 PM gkokola...@pm.me wrote: > > > On Friday, March 12, 2021 3:45 AM, Amit Langote amitlangot...@gmail.com > > > wrote: > > > > By the way, the test case added by commit 927f453a94106 does exercise > > > > the code added by this patch, but as I said in the last paragraph, we > > > > can't verify that by inspecting EXPLAIN output. > > > > > > I never doubted that. However, there is a difference. The current patch > > > changes the query to be executed in the remote from: > > > INSERT INTO <snip> VALUES ($1); > > > to: > > > INSERT INTO <snip> VALUES ($1), ($2) ... ($n); > > > When this patch gets in, it would be very helpful to know that subsequent > > > code changes will not cause regressions. So I was wondering if there is > > > a way to craft a test case that would break for the code in 927f453a94106 > > > yet succeed with the current patch. > > > > The test case "works" both before and after the patch, with the > > difference being in the form of the remote query. It seems to me > > though that you do get that. > > > > > I attach version 2 of my small reproduction. I am under the impression > > > that > > > in this version, examining the value of cmin in the remote table should > > > give an indication of whether the remote received a multiple insert > > > queries > > > with a single value, or a single insert query with multiple values. > > > Or is this a wrong assumption of mine? > > > > No, I think you have a good idea here. > > > > I've adjusted that test case to confirm that the batching indeed works > > by checking cmin of the moved rows, as you suggest. Please check the > > attached updated patch. > > Excellent. The patch in the current version with the added test seems > ready to me.
Thanks for quickly checking that. > I would still vote to have accessor functions instead of exposing the > whole PartitionTupleRouting struct, but I am not going to hold a too > strong stance about it. I as well, although I would wait for others to chime in before updating the patch that way. -- Amit Langote EDB: http://www.enterprisedb.com
