Feedback-ID: ie3de48e3:Fastmail
Date: Tue, 13 Jan 2026 12:38:39 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Mario =?utf-8?B?R29uesOhbGV6?= Troncoso <gonzalemario@gmail.com>, 
	Chao Li <li.evan.chao@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, 
	Jacob Brazeal <jacob.brazeal@gmail.com>, John Naylor <johncnaylorls@gmail.com>
Subject: Re: [PATCH} Move instrumentation structs
Message-ID: <202601131123.vxjoh2uwgavb@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAApHDvqPmpa53jcTmfU8arFFm7=hB5cFoXX5dcUH=1qV0tRFHA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/202601131123.vxjoh2uwgavb%40alvherre.pgsql>
Precedence: bulk

On 2026-Jan-13, David Rowley wrote:

> There's a bit of conflicting opinion going on here with the changes
> made in 128897b101e0.
> 
> "false when it's value for in-memory space"
> 
> I think the differing opinions depend on what you fill in the missing
> words with in the badly written English. If you read "value" as "the
> value", then "it's" is correct. Whereas if you read it as "value is",
> then "its" is correct. I assume you didn't read it the same way as
> John did.

Yeah, if the "is" had been there I wouldn't have made the change.  I
mean, this would have been correct:

   true when maxSpace is value for on-disk
   space, false when its value is for in-memory space

I debated whether to add the "is" or change the "its" to "it's" and
decided that the smaller change was better.  But I agree that a bigger
rewording may be a better solution.

> +       bool            isMaxSpaceDisk; /* true when the maxSpace value tracking is
> +                                        * on-disk space, false means it's tracking
> +                                        * memory space */

Sounds about right, though I think we can shave some words off that.
How about

    int64       maxSpace;       /* maximum amount of space occupied among sort
                                 * of groups, either in-memory or on-disk */
    bool        isMaxSpaceDisk; /* true when maxSpace tracks on-disk space, false
                                 * means in-memory */
?

(CCed Jacob as author and John as committer of that change, in case they
care to comment.)

FWIW the original wording comes from d2d8a229bc58.


Incidentally, I notice that tuplesort_updatemax() has this comment:

    /*
     * Note: it might seem we should provide both memory and disk usage for a
     * disk-based sort.  However, the current code doesn't track memory space
     * accurately once we have begun to return tuples to the caller (since we
     * don't account for pfree's the caller is expected to do), so we cannot
     * rely on availMem in a disk sort.  This does not seem worth the overhead
     * to fix.  Is it worth creating an API for the memory context code to
     * tell us how much is actually used in sortcontext?
     */

... and I think we already have the API that this comment hypothesizes
about.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La virtud es el justo medio entre dos defectos" (Aristóteles)