RE: Allow batched insert during cross-partition updates
Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Zhihong Yu <zyu@yugabyte.com>, "gkokolatos@pm.me" <gkokolatos@pm.me>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-05-07T09:39:37Z
Lists: pgsql-hackers
> > Thanks! It looks good!
>
> Thanks for checking. I'll mark this as RfC.
Hi,
The patch cannot be applied to the latest head branch, it will be nice if you can rebase it.
And when looking into the patch, I have some comments on it.
1)
IIRC, After the commit c5b7ba4, the initialization of mt_partition_tuple_routing was postponed out of ExecInitModifyTable.
So, the following if-test use "proute" which is initialized at the beginning of the ExecModifyTable() could be out of date.
And the regression test of postgres_fdw failed with the patch after the commit c5b7ba4.
+ * If the query's main target relation is a partitioned table, any inserts
+ * would have been performed using tuple routing, so use the appropriate
+ * set of target relations. Note that this also covers any inserts
+ * performed during cross-partition UPDATEs that would have occurred
+ * through tuple routing.
*/
if (proute)
...
It seems we should get the mt_partition_tuple_routing just before the if-test.
2)
+ foreach(lc, estate->es_opened_result_relations)
+ {
+ resultRelInfo = lfirst(lc);
+ if (resultRelInfo &&
I am not sure do we need to check if resultRelInfo == NULL because the the existing code did not check it.
And if we need to check it, it might be better use "if (resultRelInfo == NULL &&..."
3)
+ if (fmstate && fmstate->aux_fmstate != NULL)
+ fmstate = fmstate->aux_fmstate;
It might be better to write like " if (fmstate != NULL && fmstate->aux_fmstate != NULL)".
Best regards,
houzj
Commits
-
Allow batching of inserts during cross-partition updates.
- 594f8d3776af 16.0 landed
-
Fix handling of pending inserts in nodeModifyTable.c.
- ffbb7e65a873 16.0 cited
-
Postpone some stuff out of ExecInitModifyTable.
- c5b7ba4e67ae 14.0 cited
-
Fix tuple routing to initialize batching only for inserts
- 927f453a9410 14.0 cited