public inbox for [email protected]  

help / color / mirror / Atom feed

From: Álvaro Herrera <[email protected]>
To: David Rowley <[email protected]>
Cc: Mario González Troncoso <[email protected]>
Cc: Chao Li <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Jacob Brazeal <[email protected]>
Cc: John Naylor <[email protected]>
Subject: Re: [PATCH} Move instrumentation structs
Date: Tue, 13 Jan 2026 12:38:39 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvqPmpa53jcTmfU8arFFm7=hB5cFoXX5dcUH=1qV0tRFHA@mail.gmail.com>

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)

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH} Move instrumentation structs
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox