Feedback-ID: ia2694551:Fastmail
Date: Wed, 7 Feb 2024 11:13:24 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	pgsql-hackers@lists.postgresql.org, David Rowley <dgrowleyml@gmail.com>
Subject: Re: pgsql: Add EXPLAIN (MEMORY) to report planner memory consumption
Message-ID: <202402071013.x6psffds5qpu@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAExHW5v3EwH6SGwewLwr2aDnJePgHYKooOZuOu0XfoGfjths9A@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202402071013.x6psffds5qpu%40alvherre.pgsql>
Precedence: bulk

Many thanks, Justin, for the post-commit review.

On 2024-Feb-06, Ashutosh Bapat wrote:

> On Tue, Feb 6, 2024 at 3:51 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > Up to now, the explain "  " (two space) format is not mixed with "=".
> >
> > And, other places which show "Memory" do not use "=".  David will
> > remember prior discussions.
> > https://www.postgresql.org/message-id/20200402054120.GC14618@telsasoft.com
> > https://www.postgresql.org/message-id/20200407042521.GH2228@telsasoft.com
> >
> >                                                  "Memory: used=%lld bytes  allocated=%lld bytes",
> > vs
> >                                                  "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: %ldkB\n",
> 
> I have used = to be consistent with Buffers usage in the same Planning section.
> 
> Are you suggesting that
> "Memory: used=%lld bytes allocated=%lld bytes",
> should be used instead of
> "Memory: used=%lld bytes  allocated=%lld bytes",
> Please notice the single vs double space.

I think using a single space here would be confusing.  It's not a
problem for show_wal_usage because that one doesn't print units.
I don't know it was you (Ashutosh) or I that put the double space, but I
considered the matter and determined that two were better.

In the new line we have two different separators (: and =) because there
are two levels of info being presented; in the show_hash_info one we
have only one type of separator.

I'm not saying this is final and definite.  I'm just saying I did
consider this whole format issue and what you see is the conclusion I
reached.  It may or may not be what Ashutosh submitted -- I don't
remember.  As committer, I almost always tweak submitted patches, and I
won't apologize for that.  Further patches to correct my mistakes and
bad decisions always welcome.

> > (Also, I first thought that "peek" should be "peak", but eventually I
> > realized that's it's as intended.)
> 
> Don't understand the context. But probably it doesn't matter.

Source code always matters.  Why would people spend so much time fixing
typos otherwise?

src/backend/commands/explain.c:
static bool peek_buffer_usage(ExplainState *es, const BufferUsage *usage);

We don't want to know what the "peak" buffer usage is, but we want to
"peek" whether buffer usage would print anything.  I did have to spent a
minute thinking what the correct spelling was, here (but my English is
almost exclusively read/written, not spoken.  Condolencies if you've had
to suffer my spoken English at some conference or whatever).  I didn't
look at the dictionary back then, but now I do:
https://www.merriam-webster.com/dictionary/peek
As Justin says, it's the right word.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"After a quick R of TFM, all I can say is HOLY CR** THAT IS COOL! PostgreSQL was
amazing when I first started using it at 7.2, and I'm continually astounded by
learning new features and techniques made available by the continuing work of
the development team."
Berend Tober, http://archives.postgresql.org/pgsql-hackers/2007-08/msg01009.php