public inbox for [email protected]  

help / color / mirror / Atom feed

From: [email protected]
To: [email protected]
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: RE: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
Date: Wed, 26 Jun 2024 10:22:23 +0000
Message-ID: <TYWPR01MB10982C38F6BD6BE4D1D5CCC5EB1D62@TYWPR01MB10982.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAFiTN-v1yDvU=X+hwfJ+55=sbgDj=_kuvbduEG-F7=BjpWcnuw@mail.gmail.com>
	<[email protected]>
	<CAA=D8a1ZGWTk0RDH-LyU==RsE-217T29eprS-NBhyj+eLHyRrQ@mail.gmail.com>
	<CAAKRu_YjBPfGp85ehY1t9NN=R9pB9k=6rztaeVkAm-OeTqUK4g@mail.gmail.com>
	<CAA=D8a1G5CgvdWLd5_VCEc5Sa3rgJftcGfTEvYqG1HnjMBQmpA@mail.gmail.com>
	<[email protected]>

Hi,

Thanks for working it! I'm interested in this feature, so I'd like to participate in the
patch review. Though I've just started looking at the patch, I have two comments
about the v6 patch. (And I want to confirm the thread active.)


1) Unify the print format of leader and worker

In show_tidbitmap_info(), the number of exact/loosy blocks of the leader and workers
are printed. I think the printed format should be same. Currently, the leader does not
print the blocks of exact/lossy with a value of 0, but the workers could even if it is 0.

IMHO, it's better to print both exact/lossy blocks if at least one of the numbers of
exact/lossy blocks is greater than 0. After all, the print logic is redundant for leader
and workers, but I thought it would be better to make it a common function.

2) Move es->workers_state check

In show_tidbitmap_info(), ExplainOpenWorker() and ExplainCloseWorker() are called
after checking es->worker_state is not NULL. However, es->workers_state seem to be
able to be NULL only for the Gather node (I see ExplainPrintPlan()). Also, reading the
comments, there is a description that each worker information needs to be hidden
when printing the plan.

Even if es->workers_state becomes NULL in BitmapHeapScan node in the future,
I think that workers' information(Heap Blocks) should not be printed. Therefore,
I think es->workers_state check should be move to the place of 
"if (planstate->pstate != NULL)" like ExplainNode(), doesn't it?

IIUC, we need to correct show_sort_info() and so on too…

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION

view thread (23+ messages)  latest in thread

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], [email protected], [email protected], [email protected]
  Subject: RE: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
  In-Reply-To: <TYWPR01MB10982C38F6BD6BE4D1D5CCC5EB1D62@TYWPR01MB10982.jpnprd01.prod.outlook.com>

* 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