Date: Tue, 31 Mar 2020 21:23:38 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: James Coleman <jtc331@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Michael Paquier <michael@paquier.xyz>,
	Rafia Sabih <rafia.pghackers@gmail.com>,
	Peter Geoghegan <pg@bowt.ie>, Simon Riggs <simon@2ndquadrant.com>,
	Shaun Thomas <shaun.thomas@2ndquadrant.com>,
	Dmitry Dolgov <9erthalion6@gmail.com>,
	Alexander Korotkov <a.korotkov@postgrespro.ru>,
	PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>,
	Andreas Karlsson <andreas@proxel.se>
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Message-ID: <20200401002338.GA11291@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <18988.1585677504@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2020-Mar-31, Tom Lane wrote:

> James Coleman <jtc331@gmail.com> writes:
> > On Tue, Mar 31, 2020 at 1:04 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Perhaps the semantics are such that that's actually sensible, but it's
> >> far from a straightforward remapping of the old enum.
> 
> > Right, I didn't see the explicit "= 0" in other enums there, so it
> > made me wonder if it was intentional to designate that one had to be
> > 0, but I guess without a comment that's a lot of inference.
> 
> It's possible that somebody meant that as an indicator that the code
> depends on palloc0() leaving the field with that value.  But if so,
> you'd soon find that out ... and an actual comment would be better,
> anyway.

git blame fingers this:

commit bf11e7ee2e3607bb67d25aec73aa53b2d7e9961b
Author:     Robert Haas <rhaas@postgresql.org>
AuthorDate: Tue Aug 29 13:22:49 2017 -0400
CommitDate: Tue Aug 29 13:26:33 2017 -0400

    Propagate sort instrumentation from workers back to leader.
    
    Up until now, when parallel query was used, no details about the
    sort method or space used by the workers were available; details
    were shown only for any sorting done by the leader.  Fix that.
    
    Commit 1177ab1dabf72bafee8f19d904cee3a299f25892 forced the test case
    added by commit 1f6d515a67ec98194c23a5db25660856c9aab944 to run
    without parallelism; now that we have this infrastructure, allow
    that again, with a little tweaking to make it pass with and without
    force_parallel_mode.
    
    Robert Haas and Tom Lane
    
    Discussion: http://postgr.es/m/CA+Tgmoa2VBZW6S8AAXfhpHczb=Rf6RqQ2br+zJvEgwJ0uoD_tQ@mail.gmail.com

I looked at the discussion thread.  That patch was first posted by
Robert at
https://postgr.es/m/CA+Tgmoa2VBZW6S8AAXfhpHczb=Rf6RqQ2br+zJvEgwJ0uoD_tQ@mail.gmail.com
without the "= 0" part; later Tom posted v2 here
https://postgr.es/m/11223.1503695532@sss.pgh.pa.us
containing the "= 0", but I see no actual discussion of that point.

I suppose it could also be important to clarify that it's 0 if it were
used as an array index of some sort, but I don't see that in 2017's
commit.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services