Re: [HACKERS] Comment on GatherPath.single_copy

[Robert Haas]

On Thu, Sep 1, 2016 at 3:15 AM, Kyotaro HORIGUCHI
 wrote:
> At Wed, 31 Aug 2016 07:26:22 -0400, Tom Lane  wrote in 
> <5934.1472642...@sss.pgh.pa.us>
>> Robert Haas  writes:
>> > On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane  wrote:
>> >> Robert, could you fix the documentation for that field so it's
>> >> intelligible?
>>
>> > Uh, maybe.  The trick, as you've already noted, is finding something
>> > better.  Maybe this:
>>
>> > -boolsingle_copy;/* path must not be executed >1x */
>> > +boolsingle_copy;/* don't execute path in multiple 
>> > processes */
>>
>> OK by me.
>>
>>   regards, tom lane
>
> Me too, thanks.

OK, changed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Comment on GatherPath.single_copy

[Kyotaro HORIGUCHI]

At Wed, 31 Aug 2016 07:26:22 -0400, Tom Lane  wrote in 
<5934.1472642...@sss.pgh.pa.us>
> Robert Haas  writes:
> > On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane  wrote:
> >> Robert, could you fix the documentation for that field so it's
> >> intelligible?
> 
> > Uh, maybe.  The trick, as you've already noted, is finding something
> > better.  Maybe this:
> 
> > -boolsingle_copy;/* path must not be executed >1x */
> > +boolsingle_copy;/* don't execute path in multiple 
> > processes */
> 
> OK by me.
> 
>   regards, tom lane

Me too, thanks.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Comment on GatherPath.single_copy

[Tom Lane]

Robert Haas  writes:
> On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane  wrote:
>> Robert, could you fix the documentation for that field so it's
>> intelligible?

> Uh, maybe.  The trick, as you've already noted, is finding something
> better.  Maybe this:

> -boolsingle_copy;/* path must not be executed >1x */
> +boolsingle_copy;/* don't execute path in multiple processes 
> */

OK by me.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Comment on GatherPath.single_copy

[Robert Haas]

On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane  wrote:
> Kyotaro HORIGUCHI  writes:
>> - boolsingle_copy;/* path must not be executed >1x */
>> + boolsingle_copy;/* path must not span on multiple 
>> processes */
>
> I agree that the existing comment sucks, but this isn't a lot better
> (and it will probably not look nice after pgindent gets done with it).
> Possibly it's too complicated to jam a reasonable explanation into the
> same-line comment, and what is needed is to expand the sentence about
> it in the comment above the struct.
>
> Robert, could you fix the documentation for that field so it's
> intelligible?

Uh, maybe.  The trick, as you've already noted, is finding something
better.  Maybe this:

-boolsingle_copy;/* path must not be executed >1x */
+boolsingle_copy;/* don't execute path in multiple processes */

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Comment on GatherPath.single_copy

[Tom Lane]

Kyotaro HORIGUCHI  writes:
> - boolsingle_copy;/* path must not be executed >1x */
> + boolsingle_copy;/* path must not span on multiple 
> processes */

I agree that the existing comment sucks, but this isn't a lot better
(and it will probably not look nice after pgindent gets done with it).
Possibly it's too complicated to jam a reasonable explanation into the
same-line comment, and what is needed is to expand the sentence about
it in the comment above the struct.

Robert, could you fix the documentation for that field so it's
intelligible?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Comment on GatherPath.single_copy

[Kyotaro HORIGUCHI]

Hello.

The comment on GatherPath.single_copy is the following.

===
/*
 * GatherPath runs several copies of a plan in parallel and collects the
 * results.  The parallel leader may also execute the plan, unless the
 * single_copy flag is set.
 */
typedef struct GatherPath
{
Pathpath;
Path   *subpath;/* path for each worker */
boolsingle_copy;/* path must not be executed >1x */
} GatherPath;
===

The ">1x" looks to me as a kind of typo but looking the comment
above the struct it came to look as "more than once (or one
copy)". But it seems to me that it would be better to be in
ordinary words.


> boolsingle_copy;/* path must not be executed multiply */

If anyone feel that it is confusing with a verb form, the
following might be better.

> bool  single_copy;/* path must not span on multiple processes */

Since anyway I cannot find a comfortable expression for this, I
attached a patch that does the last one.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index e4cfc44..ed9c71f 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -312,6 +312,7 @@ gather_getnext(GatherState *gatherstate)
 
 		if (gatherstate->need_to_scan_locally)
 		{
+			elog(LOG, "EXECLOCAL");
 			outerTupleSlot = ExecProcNode(outerPlan);
 
 			if (!TupIsNull(outerTupleSlot))
@@ -385,15 +386,18 @@ gather_readnext(GatherState *gatherstate)
 
 		/* Have we visited every (surviving) TupleQueueReader? */
 		nvisited++;
+		elog(LOG, "NEXT");
 		if (nvisited >= gatherstate->nreaders)
 		{
 			/*
 			 * If (still) running plan locally, return NULL so caller can
 			 * generate another tuple from the local copy of the plan.
 			 */
+			elog(LOG, "WAIT0");
 			if (gatherstate->need_to_scan_locally)
 return NULL;
 
+			elog(LOG, "WAIT");
 			/* Nothing to do except wait for developments. */
 			WaitLatch(MyLatch, WL_LATCH_SET, 0);
 			ResetLatch(MyLatch);
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index fcfb0d4..b6b9779 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -1189,7 +1189,7 @@ typedef struct GatherPath
 {
 	Path		path;
 	Path	   *subpath;		/* path for each worker */
-	bool		single_copy;	/* path must not be executed >1x */
+	bool		single_copy;	/* path must not span on multiple processes */
 } GatherPath;
 
 /*

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers