On Tue, Apr 6, 2021 at 3:08 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Mon, Apr 5, 2021 at 1:16 AM Zhihong Yu <z...@yugabyte.com> wrote: > > > > Hi, > > In the description: > > > > cross-partition update of partitioned tables can't use batching > > because ExecInitRoutingInfo() which initializes the insert target > > > > 'which' should be dropped since 'because' should start a sentence. > > > > +-- Check that batched inserts also works for inserts made during > > > > inserts also works -> inserts also work > > > > + Assert(node->rootResultRelInfo->ri_RelationDesc->rd_rel->relkind == > > + RELKIND_PARTITIONED_TABLE); > > > > The level of nested field accesses is quite deep. If the assertion fails, > > it would be hard to know which field is null. > > Maybe use several assertions: > > Assert(node->rootResultRelInfo) > > Assert(node->rootResultRelInfo->ri_RelationDesc) > > Assert(node->rootResultRelInfo->ri_RelationDesc->rd_rel->relkind == > > ... > > Thanks for taking a look at this. > > While I agree about having the 1st Assert you suggest, I don't think > this code needs the 2nd one, because its failure would very likely be > due to a problem in some totally unrelated code. > > Updated patch attached. I had to adjust the test case a little bit to > account for the changes of 86dc90056d, something I failed to notice > yesterday. Also, I expanded the test case added in postgres_fdw.sql a > bit to show the batching in action more explicitly.
Some minor comments: 1) don't we need order by clause for the selects in the tests added? +SELECT tableoid::regclass, * FROM batch_cp_upd_test; +SELECT cmin, * FROM batch_cp_upd_test1; 2) typo - it is "should" not "shoud" +-- cmin shoud be different across rows, because each one would be inserted 3) will the cmin in the output always be the same? +SELECT cmin, * FROM batch_cp_upd_test3; With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com