On Thu, Dec 24, 2020 at 1:07 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> > You could change intoclause_len = strlen(intoclausestr) to
> > strlen(intoclausestr) + 1 and use intoclause_len in the remaining
> > places. We can avoid the +1 in the other places.
On Thu, Dec 24, 2020 at 1:07 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> > You could change intoclause_len = strlen(intoclausestr) to
> > strlen(intoclausestr) + 1 and use intoclause_len in the remaining
> > places. We can avoid the +1 in the other places.
On Fri, Dec 25, 2020 at 10:04 AM Amit Kapila wrote:
>
> On Fri, Dec 25, 2020 at 9:54 AM Bharath Rupireddy
> wrote:
> >
> > On Fri, Dec 25, 2020 at 7:12 AM vignesh C wrote:
> > > On Thu, Dec 24, 2020 at 11:29 AM Amit Kapila
> > > wrote:
> > > >
> > > > On Thu, Dec 24, 2020 at 10:25 AM vignesh
On Fri, Dec 25, 2020 at 9:54 AM Bharath Rupireddy
wrote:
>
> On Fri, Dec 25, 2020 at 7:12 AM vignesh C wrote:
> > On Thu, Dec 24, 2020 at 11:29 AM Amit Kapila
> > wrote:
> > >
> > > On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> > > >
> > > > On Tue, Dec 22, 2020 at 2:16 PM Bharath
On Fri, Dec 25, 2020 at 9:54 AM Bharath Rupireddy
wrote:
>
> On Fri, Dec 25, 2020 at 7:12 AM vignesh C wrote:
> > On Thu, Dec 24, 2020 at 11:29 AM Amit Kapila
> > wrote:
> > >
> > > On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> > > >
> > > > On Tue, Dec 22, 2020 at 2:16 PM Bharath
On Fri, Dec 25, 2020 at 7:12 AM vignesh C wrote:
> On Thu, Dec 24, 2020 at 11:29 AM Amit Kapila wrote:
> >
> > On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> > >
> > > On Tue, Dec 22, 2020 at 2:16 PM Bharath Rupireddy
> > > wrote:
> > > >
> > > > On Tue, Dec 22, 2020 at 12:32 PM Bharath
On Thu, Dec 24, 2020 at 11:29 AM Amit Kapila
wrote:
>
> On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
> >
> > On Tue, Dec 22, 2020 at 2:16 PM Bharath Rupireddy
> > wrote:
> > >
> > > On Tue, Dec 22, 2020 at 12:32 PM Bharath Rupireddy
> > > Attaching v14 patch set that has above changes.
On Thu, Dec 24, 2020 at 10:25 AM vignesh C wrote:
>
> On Tue, Dec 22, 2020 at 2:16 PM Bharath Rupireddy
> wrote:
> >
> > On Tue, Dec 22, 2020 at 12:32 PM Bharath Rupireddy
> > Attaching v14 patch set that has above changes. Please consider this
> > for further review.
> >
>
> Few comments:
> In
On Tue, Dec 22, 2020 at 2:16 PM Bharath Rupireddy
wrote:
>
> On Tue, Dec 22, 2020 at 12:32 PM Bharath Rupireddy
> Attaching v14 patch set that has above changes. Please consider this
> for further review.
>
Few comments:
In the below case, should create be above Gather?
postgres=# explain
On Mon, Dec 21, 2020 at 8:16 AM Hou, Zhijie wrote:
> The cfbost seems complains about the testcase:
>
> Command exited with code 1
> perl dumpregr.pl
> === $path ===\ndiff -w -U3
> C:/projects/postgresql/src/test/regress/expected/write_parallel.out
>
Hi
The cfbost seems complains about the testcase:
Command exited with code 1
perl dumpregr.pl
=== $path ===\ndiff -w -U3
C:/projects/postgresql/src/test/regress/expected/write_parallel.out
C:/projects/postgresql/src/test/regress/results/write_parallel.out
---
On Wed, Dec 16, 2020 at 12:06 PM Bharath Rupireddy
wrote:
>
> On Tue, Dec 15, 2020 at 5:53 PM Bharath Rupireddy
> wrote:
> > I'm merging them to the original patch set and adding the test cases
> > to cover these cases. I will post the updated patch set soon.
>
> Attaching v12 patch set.
>
>
On Tue, Dec 15, 2020 at 5:48 PM Hou, Zhijie wrote:
> > A little explanation about how to push down the ctas info in append.
> >
> > 1. about how to ignore tuple cost in this case.
> > IMO, it create gather path under append like the following:
> > query_planner
> > -make_one_rel
> >
> From: Hou, Zhijie [mailto:houzj.f...@cn.fujitsu.com]
> Sent: Tuesday, December 15, 2020 7:30 PM
> To: Bharath Rupireddy
> Cc: Amit Kapila ; Luc Vlaming ;
> PostgreSQL-development ; Zhihong Yu
> ; Dilip Kumar
> Subject: RE: Parallel Inserts in CREATE TABLE AS
>
>
> Thanks for the append use case.
>
> Here's my analysis on pushing parallel inserts down even in case the top
> node is Append.
>
> For union cases which need to remove duplicate tuples, we can't push the
> inserts or CTAS dest receiver down. If I'm not wrong, Append node is not
> doing
On Tue, Dec 15, 2020 at 2:06 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 14, 2020 at 6:08 PM Hou, Zhijie wrote:
> > Currently with the patch, we can allow parallel CTAS when topnode is Gather.
> > When top-node is Append and Gather is the sub-node of Append, I think we
> > can still enable
> >
On Mon, Dec 14, 2020 at 6:08 PM Hou, Zhijie wrote:
> Currently with the patch, we can allow parallel CTAS when topnode is Gather.
> When top-node is Append and Gather is the sub-node of Append, I think we can
> still enable
> Parallel CTAS by pushing Parallel CTAS down to the sub-node Gather,
For set_append_rel_size(), it seems this is the difference
between query_level != 1 and query_level == 1:
+ (root->parent_root->parse->CTASParallelInsInfo &
CTAS_PARALLEL_INS_IGN_TUP_COST_APPEND) &&
Maybe extract the common conditions into its own expression / variable so
that
On Mon, Dec 14, 2020 at 4:06 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 10, 2020 at 7:20 PM Bharath Rupireddy
> wrote:
> > On Thu, Dec 10, 2020 at 5:19 PM Dilip Kumar wrote:
> > > > > > + allow = ps && IsA(ps, GatherState) &&
> > > > > > !ps->ps_ProjInfo &&
> > > > > > +
Hi
> Attaching v11 patch set. Please review it further.
Currently with the patch, we can allow parallel CTAS when topnode is Gather.
When top-node is Append and Gather is the sub-node of Append, I think we can
still enable
Parallel CTAS by pushing Parallel CTAS down to the sub-node Gather,
On Thu, Dec 10, 2020 at 7:20 PM Bharath Rupireddy
wrote:
> On Thu, Dec 10, 2020 at 5:19 PM Dilip Kumar wrote:
> > > > > + allow = ps && IsA(ps, GatherState) &&
> > > > > !ps->ps_ProjInfo &&
> > > > > + plannedstmt->parallelModeNeeded &&
> > > > > +
On Thu, Dec 10, 2020 at 5:19 PM Dilip Kumar wrote:
> > > > + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo
> > > > &&
> > > > + plannedstmt->parallelModeNeeded &&
> > > > + plannedstmt->planTree &&
> > > > +
On Thu, Dec 10, 2020 at 5:00 PM Bharath Rupireddy
wrote:
>
> On Thu, Dec 10, 2020 at 4:49 PM Dilip Kumar wrote:
> > > + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
> > > + plannedstmt->parallelModeNeeded &&
> > > +
On Thu, Dec 10, 2020 at 4:49 PM Dilip Kumar wrote:
> > + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo
&&
> > + plannedstmt->parallelModeNeeded &&
> > + plannedstmt->planTree &&
> > +
On Thu, Dec 10, 2020 at 3:59 PM Hou, Zhijie wrote:
>
> Hi
>
> + allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
> + plannedstmt->parallelModeNeeded &&
> + plannedstmt->planTree &&
> +
Hi
+ allow = ps && IsA(ps, GatherState) && !ps->ps_ProjInfo &&
+ plannedstmt->parallelModeNeeded &&
+ plannedstmt->planTree &&
+ IsA(plannedstmt->planTree, Gather) &&
+
On Thu, Dec 10, 2020 at 7:48 AM Zhihong Yu wrote:
> + if (!OidIsValid(col->collOid) &&
> + type_is_collatable(col->typeName->typeOid))
> + ereport(ERROR,
> ...
> + attrList = lappend(attrList, col);
>
> Should attrList be freed when ereport is
Hi,
+ if (!OidIsValid(col->collOid) &&
+ type_is_collatable(col->typeName->typeOid))
+ ereport(ERROR,
...
+ attrList = lappend(attrList, col);
Should attrList be freed when ereport is called ?
+ query->CTASParallelInsInfo &=
On Wed, Dec 9, 2020 at 10:16 AM Dilip Kumar wrote:
>
> On Tue, Dec 8, 2020 at 6:24 PM Hou, Zhijie wrote:
> >
> > > > I'm not quite sure how to address this. Can we not allow the planner
> > > > to consider that the select is for CTAS and check only after the
> > > > planning is done for the
On Tue, Dec 8, 2020 at 6:24 PM Hou, Zhijie wrote:
>
> > > I'm not quite sure how to address this. Can we not allow the planner
> > > to consider that the select is for CTAS and check only after the
> > > planning is done for the Gather node and other checks?
> > >
> >
> > IIUC, you are saying
On Tue, Dec 8, 2020 at 6:36 PM Bharath Rupireddy
wrote:
>
> On Tue, Dec 8, 2020 at 6:24 PM Hou, Zhijie wrote:
> >
> > > > I'm not quite sure how to address this. Can we not allow the planner
> > > > to consider that the select is for CTAS and check only after the
> > > > planning is done for the
On Tue, Dec 8, 2020 at 6:24 PM Hou, Zhijie wrote:
>
> > > I'm not quite sure how to address this. Can we not allow the planner
> > > to consider that the select is for CTAS and check only after the
> > > planning is done for the Gather node and other checks?
> > >
> >
> > IIUC, you are saying
> > I'm not quite sure how to address this. Can we not allow the planner
> > to consider that the select is for CTAS and check only after the
> > planning is done for the Gather node and other checks?
> >
>
> IIUC, you are saying that we should not influence the cost of gather node
> even when
On Mon, Dec 7, 2020 at 7:04 PM Bharath Rupireddy
wrote:
>
> I'm not quite sure how to address this. Can we not allow the planner
> to consider that the select is for CTAS and check only after the
> planning is done for the Gather node and other checks?
>
IIUC, you are saying that we should not
On Mon, Dec 7, 2020 at 7:04 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 7, 2020 at 5:25 PM Amit Kapila wrote:
> >
> > On Mon, Dec 7, 2020 at 4:20 PM Bharath Rupireddy
> > wrote:
> > >
> > > On Mon, Dec 7, 2020 at 4:04 PM Amit Kapila
> > > wrote:
> > > >
> > > > What is the need of checking
On Mon, Dec 7, 2020 at 5:25 PM Amit Kapila wrote:
>
> On Mon, Dec 7, 2020 at 4:20 PM Bharath Rupireddy
> wrote:
> >
> > On Mon, Dec 7, 2020 at 4:04 PM Amit Kapila wrote:
> > >
> > > What is the need of checking query_level when 'isForCTAS' is set only
> > > when Gather is a top-node?
> > >
> >
On Mon, Dec 7, 2020 at 4:20 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 7, 2020 at 4:04 PM Amit Kapila wrote:
> >
> > What is the need of checking query_level when 'isForCTAS' is set only
> > when Gather is a top-node?
> >
>
> isForCTAS is getting set before pg_plan_query() which is being used
On Mon, Dec 7, 2020 at 3:44 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 7, 2020 at 2:55 PM Amit Kapila wrote:
> >
> > On Mon, Dec 7, 2020 at 11:32 AM Hou, Zhijie
> > wrote:
> > >
> > > + if (!(root->parse->isForCTAS &&
> > > + root->query_level == 1))
> > > +
On Mon, Dec 7, 2020 at 2:55 PM Amit Kapila wrote:
>
> On Mon, Dec 7, 2020 at 11:32 AM Hou, Zhijie wrote:
> >
> > Hi
> >
> > + /*
> > +* Flag to let the planner know that the SELECT query is for CTAS.
> > This is
> > +* used to calculate the tuple transfer cost from workers
On Mon, Dec 7, 2020 at 11:32 AM Hou, Zhijie wrote:
>
> Hi
>
> + /*
> +* Flag to let the planner know that the SELECT query is for CTAS.
> This is
> +* used to calculate the tuple transfer cost from workers to gather
> node(in
> +* case parallelism kicks in for the
Hi
+ /*
+* Flag to let the planner know that the SELECT query is for CTAS. This
is
+* used to calculate the tuple transfer cost from workers to gather
node(in
+* case parallelism kicks in for the SELECT part of the CTAS), to zero
as
+* each worker will
Thanks for the comments.
On Mon, Dec 7, 2020 at 8:56 AM Zhihong Yu wrote:
>
>
> + (void) SetCurrentCommandIdUsedForWorker();
> + myState->output_cid = GetCurrentCommandId(false);
>
> SetCurrentCommandIdUsedForWorker already has void as return type. The
> '(void)' is not needed.
>
Hi, Bharath :
+ (void) SetCurrentCommandIdUsedForWorker();
+ myState->output_cid = GetCurrentCommandId(false);
SetCurrentCommandIdUsedForWorker already has void as return type. The
'(void)' is not needed.
+* rd_createSubid is marked invalid, otherwise, the table is
+
Thanks Amit for the review comments.
On Sat, Dec 5, 2020 at 4:27 PM Amit Kapila wrote:
>
> > If I'm not wrong, I think currently we have no exec nodes for DDLs.
> > I'm not sure whether we would like to introduce one for this.
>
> Yeah, I am also not in favor of having an executor node for CTAS
On Mon, Nov 30, 2020 at 10:43 AM Bharath Rupireddy
wrote:
>
> On Fri, Nov 27, 2020 at 1:07 PM Luc Vlaming wrote:
> >
> > Disclaimer: I have by no means throughly reviewed all the involved parts
> > and am probably missing quite a bit of context so if I understood parts
> > wrong or they have
On Fri, Nov 27, 2020 at 1:07 PM Luc Vlaming wrote:
>
> Disclaimer: I have by no means throughly reviewed all the involved parts
> and am probably missing quite a bit of context so if I understood parts
> wrong or they have been discussed before then I'm sorry. Most notably
> the whole situation
On 25-11-2020 03:40, Bharath Rupireddy wrote:
On Tue, Nov 24, 2020 at 4:43 PM Hou, Zhijie wrote:
I'm very interested in this feature,
and I'm looking at the patch, here are some comments.
Thanks for the review.
How about the following style:
On Fri, Nov 27, 2020 at 11:57 AM Hou, Zhijie wrote:
>
> > Thanks a lot for the use case. Yes with the current patch table will lose
> > data related to the subplan. On analyzing further, I think we can not allow
> > parallel inserts in the cases when the Gather node has some projections
> > to
Hi,
> > I took a deep look at the projection logic.
> > In most cases, you are right that Gather node does not need projection.
> >
> > In some rare cases, such as Subplan (or initplan I guess).
> > The projection will happen in Gather node.
> >
> > The example:
> >
> > Create table test(i int);
On Thu, Nov 26, 2020 at 12:15 PM Hou, Zhijie wrote:
>
> I took a deep look at the projection logic.
> In most cases, you are right that Gather node does not need projection.
>
> In some rare cases, such as Subplan (or initplan I guess).
> The projection will happen in Gather node.
>
> The
Hi ,
> On Thu, Nov 26, 2020 at 7:47 AM Hou, Zhijie
> wrote:
> >
> > Hi,
> >
> > I have an issue about the following code:
> >
> > econtext = node->ps.ps_ExprContext;
> > ResetExprContext(econtext);
> >
> > + if (ISCTAS(node->ps.intoclause))
> > + {
> > +
On Thu, Nov 26, 2020 at 7:47 AM Hou, Zhijie wrote:
>
> Hi,
>
> I have an issue about the following code:
>
> econtext = node->ps.ps_ExprContext;
> ResetExprContext(econtext);
>
> + if (ISCTAS(node->ps.intoclause))
> + {
> + ExecParallelInsertInCTAS(node);
Hi,
I have an issue about the following code:
econtext = node->ps.ps_ExprContext;
ResetExprContext(econtext);
+ if (ISCTAS(node->ps.intoclause))
+ {
+ ExecParallelInsertInCTAS(node);
+ return NULL;
+ }
/* If no projection
On Tue, Nov 24, 2020 at 4:43 PM Hou, Zhijie wrote:
>
> I'm very interested in this feature,
> and I'm looking at the patch, here are some comments.
>
Thanks for the review.
>
> How about the following style:
>
> if(TupIsNull(outerTupleSlot))
> Break;
>
>
Hi,
I'm very interested in this feature,
and I'm looking at the patch, here are some comments.
1.
+ if (!TupIsNull(outerTupleSlot))
+ {
+ (void)
node->ps.dest->receiveSlot(outerTupleSlot, node->ps.dest);
+
On Mon, Oct 19, 2020 at 10:47 PM Bharath Rupireddy
wrote:
>
> Attaching v3 patch herewith.
>
> I'm done with all the open points in my list. Please review the v3 patch and
> provide comments.
>
Attaching v4 patch, rebased on the latest master 68b1a4877e. Also,
added this feature to commitfest -
On Thu, Oct 15, 2020 at 3:18 PM Amit Kapila wrote:
>
> > > > 1. How to represent the parallel insert for CTAS in explain plans?
The
> > > > explain CTAS shows the plan for only the SELECT part. How about
having
> > > > some textual info along with the Gather node? I'm not quite sure on
> > > >
On 16.10.20 08:23, Bharath Rupireddy wrote:
On Fri, Oct 16, 2020 at 11:33 AM Luc Vlaming wrote:
Really looking forward to this ending up in postgres as I think it's a
very nice improvement.
Whilst reviewing your patch I was wondering: is there a reason you did
not introduce a batch insert in
On Fri, Oct 16, 2020 at 11:33 AM Luc Vlaming wrote:
>
> Really looking forward to this ending up in postgres as I think it's a
> very nice improvement.
>
> Whilst reviewing your patch I was wondering: is there a reason you did
> not introduce a batch insert in the destreceiver for the CTAS? For
On 14.10.20 11:16, Bharath Rupireddy wrote:
On Tue, Oct 6, 2020 at 10:58 AM Amit Kapila wrote:
Yes we do a bunch of catalog changes related to the created new table.
We will have both the txn id and command id assigned when catalogue
changes are being made. But, right after the table is
On Thu, Oct 15, 2020 at 9:14 AM Bharath Rupireddy
wrote:
>
> On Wed, Oct 14, 2020 at 6:16 PM Amit Kapila wrote:
> >
> > > For prepared statements, the parallelism will not be picked and so is
> > > parallel insertion.
> >
> > Hmm, I am not sure what makes you say this statement. The parallelism
On Wed, Oct 14, 2020 at 6:16 PM Amit Kapila wrote:
>
> > If somebody expects to preserve the order of the tuples that are
> > coming from GatherMerge node of the select part in CTAS or SELECT INTO
> > while inserting, now if parallelism is allowed, that may not be the
> > case i.e. the order of
On Wed, Oct 14, 2020 at 2:46 PM Bharath Rupireddy
wrote:
>
> On Tue, Oct 6, 2020 at 10:58 AM Amit Kapila wrote:
> >
> >
> > While skimming through the patch, a small thing I noticed:
> > + /*
> > + * SELECT part of the CTAS is parallelizable, so we can make
> > + * each parallel worker insert
On Tue, Oct 6, 2020 at 10:58 AM Amit Kapila wrote:
>
> > Yes we do a bunch of catalog changes related to the created new table.
> > We will have both the txn id and command id assigned when catalogue
> > changes are being made. But, right after the table is created in the
> > leader, the command
On Mon, Sep 28, 2020 at 3:58 PM Bharath Rupireddy
wrote:
>
> Thanks Andres for the comments.
>
> On Thu, Sep 24, 2020 at 8:11 AM Andres Freund wrote:
> >
> > > The design:
> >
> > I think it'd be good if you could explain a bit more why you think this
> > safe to do in the way you have done it.
Thanks Andres for the comments.
On Thu, Sep 24, 2020 at 8:11 AM Andres Freund wrote:
>
> > The design:
>
> I think it'd be good if you could explain a bit more why you think this
> safe to do in the way you have done it.
>
> E.g. from a quick scroll through the patch, there's not even a comment
Hi,
On 2020-09-23 17:20:20 +0530, Bharath Rupireddy wrote:
> The idea of this patch is to allow the leader and each worker insert the
> tuples in parallel if the SELECT part of the CTAS is parallelizable.
Cool!
> The design:
I think it'd be good if you could explain a bit more why you think
>
> [1] - For table_multi_insert() in CTAS, I used an in-progress patch available
> at
> https://www.postgresql.org/message-id/CAEET0ZG31mD5SWjTYsAt0JTLReOejPvusJorZ3kGZ1%3DN1AC-Fw%40mail.gmail.com
> [2] - Table with 2 integer columns, 100million tuples, with leader
> participation,with default
101 - 168 of 168 matches
Mail list logo
