From: | "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Luc Vlaming <luc(at)swarm64(dot)com>, vignesh C <vignesh21(at)gmail(dot)com> |
Subject: | RE: Parallel Inserts in CREATE TABLE AS |
Date: | 2021-01-06 05:36:26 |
Message-ID: | f12c78d9c1b04cb081b371919da8d27c@G08CNEXMBPEKD05.g08.fujitsu.local |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> > For v20-0001-Parallel-Inserts-in-CREATE-TABLE-AS.patch :
> >
> > ParallelInsCmdEstimate :
> >
> > + Assert(pcxt && ins_info &&
> > + (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS));
> > +
> > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> >
> > Sinc the if condition is covered by the assertion, I wonder why the if
> check is still needed.
> >
> > Similar comment for SaveParallelInsCmdFixedInfo and
> > SaveParallelInsCmdInfo
>
> Thanks.
>
> The idea is to have assertion with all the expected ins_cmd types, and then
> later to have selective handling for different ins_cmds. For example, if
> we add (in future) parallel insertion in Refresh Materialized View, then
> the code in those functions will be something
> like:
>
> +static void
> +ParallelInsCmdEstimate(ParallelContext *pcxt, ParallelInsertCmdKind
> ins_cmd,
> + void *ins_info)
> +{
> + Assert(pcxt && ins_info &&
> + (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS ||
> + (ins_cmd == PARALLEL_INSERT_CMD_REFRESH_MAT_VIEW));
> +
> + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> + {
> +
> + }
> + else if (ns_cmd == PARALLEL_INSERT_CMD_REFRESH_MAT_VIEW)
> + {
> +
> + }
>
> Similarly for other functions as well.
I think it makes sense.
And if the check about ' ins_cmd == xxx1 || ins_cmd == xxx2' may be used in some places,
How about define a generic function with some comment to mention the purpose.
An example in INSERT INTO SELECT patch:
+/*
+ * IsModifySupportedInParallelMode
+ *
+ * Indicates whether execution of the specified table-modification command
+ * (INSERT/UPDATE/DELETE) in parallel-mode is supported, subject to certain
+ * parallel-safety conditions.
+ */
+static inline bool
+IsModifySupportedInParallelMode(CmdType commandType)
+{
+ /* Currently only INSERT is supported */
+ return (commandType == CMD_INSERT);
+}
Best regards,
houzj
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2021-01-06 05:36:42 | Re: Cirrus CI (Windows help wanted) |
Previous Message | Amit Kapila | 2021-01-06 05:06:11 | Re: Single transaction in the tablesync worker? |