On Tue, Apr 6, 2021 at 3:08 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Mon, Apr 5, 2021 at 1:16 AM Zhihong Yu <zyu@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
