Comment on GatherPath.single_copy
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
{
Path path;
Path *subpath; /* path for each worker */
bool single_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.
bool single_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
Attachments:
modify_comment_for_Gatherpath_single_copy.difftext/x-patch; charset=us-asciiDownload
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;
/*
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
- bool single_copy; /* path must not be executed >1x */ + bool single_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
On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
- bool single_copy; /* path must not be executed >1x */ + bool single_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:
- bool single_copy; /* path must not be executed >1x */
+ bool single_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
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> 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:
- bool single_copy; /* path must not be executed >1x */ + bool single_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
At Wed, 31 Aug 2016 07:26:22 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <5934.1472642782@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> 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:- bool single_copy; /* path must not be executed >1x */ + bool single_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
On Thu, Sep 1, 2016 at 3:15 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
At Wed, 31 Aug 2016 07:26:22 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <5934.1472642782@sss.pgh.pa.us>
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Aug 30, 2016 at 6:38 PM, Tom Lane <tgl@sss.pgh.pa.us> 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:- bool single_copy; /* path must not be executed >1x */ + bool single_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