Misleading comment about single_copy, and some bikeshedding

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Misleading comment about single_copy, and some bikeshedding
Date: 2019-06-24 05:20:00
Message-ID: CA+hUKGJEaZJYezXAOutuiWT+fxCA44+oKtVPAND2ubLiigR=-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

/*
* GatherMergePath runs several copies of a plan in parallel and collects
* the results, preserving their common sort order. For gather merge, the
* parallel leader always executes the plan too, so we don't need single_copy.
*/
typedef struct GatherMergePath

The second sentence is not true as of commit e5253fdc, and the
attached patch removes it.

Even before that commit, the comment was a bit circular: the reason
GatherMergePath doesn't need a single_copy field is because
force_parallel_mode specifically means "try to stick a Gather node on
top in a test mode with one worker and no leader participation", and
this isn't a Gather node.

Hmm. I wonder if we should rename force_parallel_mode to
force_gather_node in v13. The current name has always seemed slightly
misleading to me; it sounds like some kind of turbo boost button but
really it's a developer-only test mode. Also, does it belong under
DEVELOPER_OPTIONS instead of QUERY_TUNING_OTHER? I'm also wondering
if the variable single_copy would be better named
no_leader_participation or single_participant. I find "copy" a
slightly strange way to refer to the number of copies *allowed to
run*, but maybe that's just me.

--
Thomas Munro
https://enterprisedb.com

Attachment Content-Type Size
0001-Remove-misleading-comment-from-pathnodes.h.patch application/octet-stream 1.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2019-06-24 08:21:46 MAKEFLAGS in non-GNU Makefile
Previous Message Michael Paquier 2019-06-24 05:10:28 Re: using explicit_bzero