Re: Parallel Inserts in CREATE TABLE AS

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

Cheers

On Mon, Jan 4, 2021 at 7:59 PM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> On Mon, Jan 4, 2021 at 7:02 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > > + if
> (IS_PARALLEL_CTAS_DEST(gstate->dest) &&
> > > + ((DR_intorel *)
> gstate->dest)->into->rel &&
> > > + ((DR_intorel *)
> gstate->dest)->into->rel->relname)
> > > why would rel and relname not be there? if no rows have been inserted?
> > > because it seems from the intorel_startup function that that would be
> > > set as soon as startup was done, which i assume (wrongly?) is always
> done?
> >
> > Actually, that into clause rel variable is always being set in the
> gram.y for CTAS, Create Materialized View and SELECT INTO (because
> qualified_name non-terminal is not optional). My bad. I just added it as a
> sanity check. Actually, it's not required.
> >
> > create_as_target:
> > qualified_name opt_column_list table_access_method_clause
> > OptWith OnCommitOption OptTableSpace
> > {
> > $$ = makeNode(IntoClause);
> > $$->rel = $1;
> > create_mv_target:
> > qualified_name opt_column_list table_access_method_clause
> opt_reloptions OptTableSpace
> > {
> > $$ = makeNode(IntoClause);
> > $$->rel = $1;
> > into_clause:
> > INTO OptTempTableName
> > {
> > $$ = makeNode(IntoClause);
> > $$->rel = $2;
> >
> > I will change the below code:
> > + if (GetParallelInsertCmdType(gstate->dest) ==
> > + PARALLEL_INSERT_CMD_CREATE_TABLE_AS &&
> > + ((DR_intorel *) gstate->dest)->into &&
> > + ((DR_intorel *) gstate->dest)->into->rel &&
> > + ((DR_intorel *)
> gstate->dest)->into->rel->relname)
> > + {
> >
> > to:
> > + if (GetParallelInsertCmdType(gstate->dest) ==
> > + PARALLEL_INSERT_CMD_CREATE_TABLE_AS)
> > + {
> >
> > I will update this in the next version of the patch set.
>
> Attaching v20 patch set that has above change in 0001 patch, note that
> 0002 to 0004 patches have no changes from v19. Please consider the v20
> patch set for further review.
>
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>