The plan sounds good.
Before the second command type is added, can you leave out the 'if (ins_cmd
== PARALLEL_INSERT_CMD_CREATE_TABLE_AS)' and keep the pair of curlies ?
You can add the if condition back when the second command type is added.
Cheers
On Tue, Jan 5, 2021 at 7:53 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> On Wed, Jan 6, 2021 at 8:19 AM Zhihong Yu <z...@yugabyte.com> wrote:
> > 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.
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>
