Subject: Re: [PATCH} Move instrumentation structs

From: Álvaro Herrera <[email-masked]>
Date: Tue, 13 Jan 2026 12:38:39 +0100

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)