Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tsN1w-001FYH-9k for pgsql-hackers@arkaria.postgresql.org; Wed, 12 Mar 2025 14:25:56 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tsN1v-006CyT-2D for pgsql-hackers@arkaria.postgresql.org; Wed, 12 Mar 2025 14:25:55 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tsN1u-006CyE-5Z for pgsql-hackers@lists.postgresql.org; Wed, 12 Mar 2025 14:25:54 +0000 Received: from fhigh-b4-smtp.messagingengine.com ([202.12.124.155]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1tsN1p-002SIA-20 for pgsql-hackers@postgresql.org; Wed, 12 Mar 2025 14:25:53 +0000 Received: from phl-compute-13.internal (phl-compute-13.phl.internal [10.202.2.53]) by mailfhigh.stl.internal (Postfix) with ESMTP id 04CA52540196; Wed, 12 Mar 2025 10:25:48 -0400 (EDT) Received: from phl-imap-09 ([10.202.2.99]) by phl-compute-13.internal (MEProxy); Wed, 12 Mar 2025 10:25:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eulerto.com; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1741789548; x=1741875948; bh=Cn34Ueg+SF 7cBu83uqY9yN/2CDOKYDJpVoef2BPmmio=; b=uZhxXpT6SrDQqdEkgCjt+NgCx5 oQfOOQ6GLO8FdhpexlzqcP7Wd3ONngUm9AjX1gqqUZzPftfFw2Vo5IOUW3IStM3A y6KptFJ9fwQm3EZdCHI3jGNG43t7rUqp695RMJLK4PN0HbnzIWqh4S+0EOPEumGY 1h/DoTM5BtBhXjw/3QySP7BnsFVa9Sf/iYxKZa2K/YHd3adbm8l/6Pmr7OIgVXO5 PPOwQXZy43IK2Tw9X8OYYkaTQSL+fQre1i88MF/3t+FigTLkldvzMWI9wzyzFclk EKjAoLyady8bPp01cRvWTXngtywbufGFfM2okGFEXRQzx8mtPh95en2pcfIw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1741789548; x=1741875948; bh=Cn34Ueg+SF7cBu83uqY9yN/2CDOKYDJpVoe f2BPmmio=; b=v/ZSIMC/f6MZyumtmZymGY29KTEvS+tsiIEQy2iM9UpN1U3BSbS ZJ4EUyv848Dwg5UmwiU/jtOsgjZaab2bzcJ3JwXEwhYcLwm6T+d98Y1PDQkKWGM5 NCO7P+Sj60tVe/fi4vHFT3Fz74Z6L2g0MpSwsAIO8BKuc4t5gT6YeXMO9NCM092f WS7lcB9FR0vTiW/KQFtGfEp5E+fcV8qeViYq3jUntmWytzHCHRGj2oYPMFUF7BXg djoah1axrGjkXsMwy7BqnqewZhWB93TtSUPWyEEuiL5Vy6hyKiHa03k+iSREV/n0 8AG7QTQyZYMlviV7MpZVuU2StbM5KDuY43A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvdehfedtucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefogg ffhffvvefkjghfufgtsegrtderreertddtnecuhfhrohhmpedfgfhulhgvrhcuvfgrvhgv ihhrrgdfuceovghulhgvrhesvghulhgvrhhtohdrtghomheqnecuggftrfgrthhtvghrnh epkeehteevgfegtdduveejheejffetuedvhfeftddvgfelkedtgfefkedthfehkedvnecu ffhomhgrihhnpegvnhhtvghrphhrihhsvggusgdrtghomhenucevlhhushhtvghrufhiii gvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegvuhhlvghrsegvuhhlvghrthhordgt ohhmpdhnsggprhgtphhtthhopeefpdhmohguvgepshhmthhpohhuthdprhgtphhtthhope hrrghfrggvlhhthhgtrgesghhmrghilhdrtghomhdprhgtphhtthhopehtohhrihhkohhs hhhirgesohhsshdrnhhtthgurghtrgdrtghomhdprhgtphhtthhopehpghhsqhhlqdhhrg gtkhgvrhhssehpohhsthhgrhgvshhqlhdrohhrgh X-ME-Proxy: Feedback-ID: i0c21471d:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id E8835780068; Wed, 12 Mar 2025 10:25:47 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface MIME-Version: 1.0 Date: Wed, 12 Mar 2025 11:25:27 -0300 From: "Euler Taveira" To: "Rafael Thofehrn Castro" , pgsql-hackers Cc: torikoshia@oss.nttdata.com Message-Id: In-Reply-To: References: <9b30b34b-ed3c-4859-a3f6-45094811780f@anayrat.info> Subject: Re: Proposal: Progressive explain Content-Type: multipart/alternative; boundary=2f95292f948b4a9f804aa8253ce0ffe1 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --2f95292f948b4a9f804aa8253ce0ffe1 Content-Type: text/plain Content-Transfer-Encoding: 7bit On Fri, Mar 7, 2025, at 5:34 PM, Rafael Thofehrn Castro wrote: > Did additional benchmarks and found issues with the patch that doesn't do execProcNode > wrapping. There are sporadic crashes with **double free or corruption (top)** > **** > So making the patch that uses the wrapper the current one. Again, giving the credits to > torikoshia as being the owner of that section of the code. > Rafael, thanks for working on it. It is a step forward in observability. I started with some performance tests and the last improvements seem to fix the overhead imposed in the initial patch version. I didn't notice any of these new function in the perf report while executing fast queries. I found a crash. It is simple to reproduce. Session A: select * from pg_stat_progress_explain; \watch 2 Session B: explain select pg_sleep(30); \watch 2 8<--------------------------------------------------------------------8< Backtrace: Core was generated by `postgres: euler postgres [lo'. Program terminated with signal SIGSEGV, Segmentation fault. #0 WrapExecProcNodeWithExplain (ps=0x7f7f7f7f7f7f7f7f) at explain.c:5401 5401 if (ps->ExecProcNodeOriginal != NULL) #0 WrapExecProcNodeWithExplain (ps=0x7f7f7f7f7f7f7f7f) at explain.c:5401 #1 0x00005624173829aa in handle_sig_alarm (postgres_signal_arg=) at timeout.c:414 #2 0x00005624173ba02c in wrapper_handler (postgres_signal_arg=14) at pqsignal.c:110 #3 #4 0x00007f20fa529e63 in epoll_wait (epfd=6, events=0x56244ef37e58, maxevents=1, timeout=-1) at ../sysdeps/unix/sysv/linux/epoll_wait.c:30 #5 0x00005624171fb02f in WaitEventSetWaitBlock (nevents=1, occurred_events=0x7ffdd9e62080, cur_timeout=-1, set=0x56244ef37dd8) at waiteventset.c:1190 #6 WaitEventSetWait (set=0x56244ef37dd8, timeout=timeout@entry=-1, occurred_events=occurred_events@entry=0x7ffdd9e62080, nevents=nevents@entry=1, wait_event_info=wait_event_info@entry=100663296) at waiteventset.c:1138 #7 0x000056241709513c in secure_read (port=0x56244eeb90e0, ptr=0x56241775a9a0 , len=8192) at be-secure.c:218 #8 0x000056241709bf2e in pq_recvbuf () at pqcomm.c:924 #9 0x000056241709ceb5 in pq_getbyte () at pqcomm.c:970 #10 0x000056241721b617 in SocketBackend (inBuf=0x7ffdd9e622a0) at postgres.c:361 #11 ReadCommand (inBuf=0x7ffdd9e622a0) at postgres.c:484 #12 PostgresMain (dbname=, username=) at postgres.c:4625 #13 0x00005624172167ed in BackendMain (startup_data=, startup_data_len=) at backend_startup.c:107 #14 0x000056241717519b in postmaster_child_launch (child_type=, child_slot=2, startup_data=startup_data@entry=0x7ffdd9e6253c, startup_data_len=startup_data_len@entry=4, client_sock=client_sock@entry=0x7ffdd9e62540) at launch_backend.c:274 #15 0x0000562417178c32 in BackendStartup (client_sock=0x7ffdd9e62540) at postmaster.c:3519 #16 ServerLoop () at postmaster.c:1688 #17 0x000056241717a6da in PostmasterMain (argc=argc@entry=1, argv=argv@entry=0x56244eeb81b0) at postmaster.c:1386 #18 0x0000562416e64f9a in main (argc=1, argv=0x56244eeb81b0) at main.c:230 8<--------------------------------------------------------------------8< You call this feature "progressive explain". My first impression is that it will only provide the execution plans for EXPLAIN commands. Instead of "progressive explain", I would suggest "query progress" that is a general database terminology. It seems natural to use "progressive explain" since you are using the explain infrastructure (including the same options -- format, settings, wal, ...) -- to print the execution plan. +CREATE VIEW pg_stat_progress_explain AS + SELECT + * + FROM pg_stat_progress_explain(true); + There is no use for the function argument. If you decide to keep this function, remove it. Why don't you use the pgstat_progress_XXX() API? Since you are using a pg_stat_progress_XXX view name I would expect using the command progress reporting infrastructure (see backend_progress.c). Maybe you could include datid and datname as the other progress reporting views. It would avoid a join to figure out what the database is. +static const struct config_enum_entry explain_format_options[] = { + {"text", EXPLAIN_FORMAT_TEXT, false}, + {"xml", EXPLAIN_FORMAT_XML, false}, + {"json", EXPLAIN_FORMAT_JSON, false}, + {"yaml", EXPLAIN_FORMAT_YAML, false}, + {NULL, 0, false} +}; Isn't it the same definition as in auto_explain.c? Use only one definition for auto_explain and this feature. You can put this struct into explain.c, use an extern definition for guc_tables.c and put a extern PGDLLIMPORT defintion into guc.h. See wal_level_options, for an example. +static const struct config_enum_entry progressive_explain_options[] = { + {"off", PROGRESSIVE_EXPLAIN_NONE, false}, + {"explain", PROGRESSIVE_EXPLAIN_EXPLAIN, false}, + {"analyze", PROGRESSIVE_EXPLAIN_ANALYZE, false}, + {"false", PROGRESSIVE_EXPLAIN_NONE, true}, + {NULL, 0, false} +}; The "analyze" is a separate option in auto_explain. Should we have 2 options? One that enable/disable this feature and another one that enable/disable analyze option. Don't the other EXPLAIN options make sense here? Like serialize and summary. TupleDesc tupDesc; /* descriptor for result tuples */ EState *estate; /* executor's query-wide state */ PlanState *planstate; /* tree of per-plan-node state */ + struct ExplainState *pe_es; /* progressive explain state if enabled */ Should you use the same name pattern here? pestate, for example. PG_LWLOCK(52, SerialControl) +PG_LWLOCK(53, ExplainHash) Could you use a specific name? Even if you keep the proposed name, you should use ProgressiveExplainHash, ProgressiveExplain or QueryProgress. +$node->init; +# Configure progressive explain to be logged immediatelly +$node->append_conf('postgresql.conf', 'progressive_explain_min_duration = 0'); +$node->start; s/immediatelly/immediately/ +typedef struct progressiveExplainHashKey +{ + int pid; /* PID */ +} progressiveExplainHashKey; + +typedef struct progressiveExplainHashEntry +{ + progressiveExplainHashKey key; /* hash key of entry - MUST BE FIRST */ + dsa_handle h; + dsa_pointer p; +} progressiveExplainHashEntry; You don't need progressiveExplainHashKey. Use pid as key directly. + /* Update shared memory with new data */ + strcpy(pe_data->plan, es->str->data); + pe_data->last_print = GetCurrentTimestamp(); I don't think last_print is accurate because it is not the time the execution plan is printed but the time it was updated. I suggest last_updated_time. +/* Flag set by timeouts to control when to print progressive explains */ +bool ProgressiveExplainPending = false; s/print/update/ There are other point that you use "print" but it is better to replace it with "update". + progressiveExplainArray = ShmemInitHash("progressive explain hash", + 50, 50, + &info, + HASH_ELEM | HASH_BLOBS); I'm wondering why you use "array" in the name. ProgressiveExplainHash is a better name. + entry->p = dsa_allocate(es->pe_a, + add_size(sizeof(progressiveExplainData), + add_size(strlen(es->str->data), + PROGRESSIVE_EXPLAIN_ALLOC_SIZE))); I think you need a better name for PROGRESSIVE_EXPLAIN_ALLOC_SIZE because it doesn't reflect what it is. PROGRESSIVE_EXPLAIN_FREE_SIZE or PROGRESSIVE_EXPLAIN_AVAIL_SIZE? Maybe you can use dshash. There are some comments that still refers to the wrong function name. +/* + * ExecProcNodeWithExplain + * ExecProcNode wrapper that initializes progressive explain + * and uwraps ExecProdNode to the original function. + */ +static TupleTableSlot * +ExecProcNodeExplain(PlanState *node) and +/* + * ExecProcNodeWithExplain + * Responsible for initialization of all structures related to progressive + * explains. + * + /* state related to progressive explains */ + struct PlanState *pe_curr_node; + struct Instrumentation *pe_local_instr; + dsa_area *pe_a; Could you add some comments saying what each of these variables are for? I didn't experiment but I was wondering if there is a way to avoid the duplicates that you added to avoid the overhead. -- Euler Taveira EDB https://www.enterprisedb.com/ --2f95292f948b4a9f804aa8253ce0ffe1 Content-Type: text/html Content-Transfer-Encoding: quoted-printable
On Fri, Mar 7, = 2025, at 5:34 PM, Rafael Thofehrn Castro wrote:
 Did additional benchmarks and= found issues with the patch that doesn't do execProcNode
= wrapping. There are sporadic crashes with double free or corr= uption (top)

So makin= g the patch that uses the wrapper the current one. Again, giving the cre= dits to
torikoshia as being the owner of that section of t= he code.


Rafael, thanks for working on it. It is a step forward in observab= ility. I
started with some performance tests and the last = improvements seem to fix the
overhead imposed in the initial patch version. I didn't notice = any of these new
fun= ction in the perf report while executing fast queries.

I found a crash. It is simple to reproduce.

Session A:

select = * from pg_stat_progress_explain;
\watch 2

Session B:

explain select pg_sleep(30);
\watch 2

8<-------------------------------------------------------------= -------8<

Backtrace:

Core was generated by `postgres: euler postgres [lo'.<= br style=3D"max-width:100%;height:auto;">
Program terminated w= ith signal SIGSEGV, Segmentation fault.
#0  WrapExecProcNodeWithExplain (ps=3D0x7f7f7f7= f7f7f7f7f) at explain.c:5401
5401 if (ps->ExecProcNodeOriginal !=3D NULL)
#0  WrapExecProcNodeWithExpla= in (ps=3D0x7f7f7f7f7f7f7f7f) at explain.c:5401
#1  0x00005624173829aa in handle_sig_ala= rm (postgres_signal_arg=3D<optimized out>) at timeout.c:414
#2  0x00005624173ba02= c in wrapper_handler (postgres_signal_arg=3D14) at pqsignal.c:110
#3  <signal handle= r called>
#4 = ; 0x00007f20fa529e63 in epoll_wait (epfd=3D6, events=3D0x56244ef37e58, m= axevents=3D1, timeout=3D-1) at ../sysdeps/unix/sysv/linux/epoll_wait.c:3= 0
#5  0x0000562= 4171fb02f in WaitEventSetWaitBlock (nevents=3D1, occurred_events=3D0x7ff= dd9e62080, cur_timeout=3D-1, set=3D0x56244ef37dd8) at waiteventset.c:119= 0
#6  WaitEvent= SetWait (set=3D0x56244ef37dd8, timeout=3Dtimeout@entry=3D-1, occurred_ev= ents=3Doccurred_events@entry=3D0x7ffdd9e62080, nevents=3Dnevents@entry=3D= 1, wait_event_info=3Dwait_event_info@entry=3D100663296) at waiteventset.= c:1138
#7  0x00= 0056241709513c in secure_read (port=3D0x56244eeb90e0, ptr=3D0x56241775a9= a0 <PqRecvBuffer>, len=3D8192) at be-secure.c:218
#8  0x000056241709bf2e in pq_re= cvbuf () at pqcomm.c:924
=
#9  0x000056241709ceb5 in pq_getbyte () at pqcomm.c:970
#10 0x000056241721b617 in = SocketBackend (inBuf=3D0x7ffdd9e622a0) at postgres.c:361
#11 ReadCommand (inBuf=3D0x7ffdd9e6= 22a0) at postgres.c:484
<= div>#12 PostgresMain (dbname=3D<optimized out>, username=3D<opt= imized out>) at postgres.c:4625
#13 0x00005624172167ed in BackendMain (startup_data=3D<= ;optimized out>, startup_data_len=3D<optimized out>) at backend= _startup.c:107
#14 0= x000056241717519b in postmaster_child_launch (child_type=3D<optimized= out>, child_slot=3D2, startup_data=3Dstartup_data@entry=3D0x7ffdd9e6= 253c, startup_data_len=3Dstartup_data_len@entry=3D4, client_sock=3Dclien= t_sock@entry=3D0x7ffdd9e62540) at launch_backend.c:274
#15 0x0000562417178c32 in BackendStar= tup (client_sock=3D0x7ffdd9e62540) at postmaster.c:3519
#16 ServerLoop () at postmaster.c:16= 88
#17 0x00005624171= 7a6da in PostmasterMain (argc=3Dargc@entry=3D1, argv=3Dargv@entry=3D0x56= 244eeb81b0) at postmaster.c:1386
#18 0x0000562416e64f9a in main (argc=3D1, argv=3D0x56244eeb= 81b0) at main.c:230
=
8<--------------= ------------------------------------------------------8<

You call this feature "progressive explain". My fir= st impression is that it
=
will only provide the execution plans for EXPLAIN commands. Instead= of
"progressive exp= lain", I would suggest "query progress" that is a general
database terminology. It seems nat= ural to use "progressive explain" since you
are using the explain infrastructure (including = the same options -- format,
settings, wal, ...) -- to print the execution plan.

+CREATE VIEW pg_stat_progress_explain AS
+    SELECT
+    = ;        *
+    FROM pg_stat_progress_expl= ain(true);
+

There is no use for the function argument. If = you decide to keep this function,
remove it.
=

Why don't you = use the pgstat_progress_XXX() API? Since you are using a
pg_stat_progress_XXX view name I wo= uld expect using the command progress
reporting infrastructure (see backend_progress.c).

Maybe you could include datid and datname = as the other progress reporting
views. It would avoid a join to figure out what the database= is.

+static const struct config_enum_e= ntry explain_format_options[] =3D {
+   {"text", EXPLAIN_FORMAT_TEXT, false},
+   {"xml", EX= PLAIN_FORMAT_XML, false},
+   {"json", EXPLAIN_FORMAT_JSON, false},
+   {"yaml", EXPLAIN_FOR= MAT_YAML, false},
+&= nbsp;  {NULL, 0, false}
+};

Isn't it the same definiti= on as in auto_explain.c? Use only one definition for
auto_explain and this feature. You can = put this struct into explain.c, use an
extern definition for guc_tables.c and put a extern P= GDLLIMPORT defintion into
guc.h. See wal_level_options, for an example.

+static const struct config_enum_entry progressive_explain_= options[] =3D {
+&nb= sp;  {"off", PROGRESSIVE_EXPLAIN_NONE, false},
+   {"explain", PROGRESSIVE_EX= PLAIN_EXPLAIN, false},
+   {"analyze", PROGRESSIVE_EXPLAIN_ANALYZE, false},
+   {"false", PR= OGRESSIVE_EXPLAIN_NONE, true},
=
+   {NULL, 0, false}
+};

The "analyze"= is a separate option in auto_explain. Should we have 2 options?
One that enable/disable thi= s feature and another one that enable/disable
analyze option.

Don't the other EXPLAIN options make sense here? Like serialize and s= ummary.

    TupleDesc&nb= sp;  tupDesc;        /* descript= or for result tuples */
<= div>    EState     *estate; &nbs= p;       /* executor's query-wide state */=
    = PlanState  *planstate;      /* tree of per= -plan-node state */
= +   struct ExplainState *pe_es; /* progressive explain state i= f enabled */

Should you use the same na= me pattern here? pestate, for example.

= PG_LWLOCK(52, SerialControl)
+PG_LWLOCK(53, ExplainHash)

Co= uld you use a specific name? Even if you keep the proposed name, you sho= uld
use ProgressiveE= xplainHash, ProgressiveExplain or QueryProgress.

<= /div>
+$node->init;
+# Configure progressive explain to be logged immediatelly
+$node->append_conf('po= stgresql.conf', 'progressive_explain_min_duration =3D 0');
+$node->start;

s/immediatelly/immediately/

+typedef struct progressiveExplainHashKey
+{
+   int       &= nbsp; pid;          &n= bsp; /* PID */
+} pr= ogressiveExplainHashKey;
=
+    
=
+typedef struct progressiveExplainHashEntry
+{
+   progressiveExplainHashKey key;  /* = hash key of entry - MUST BE FIRST */
+   dsa_handle  h;
+   dsa_pointer p;
+} progressiveExplainHashEntry;<= br style=3D"max-width:100%;height:auto;">

You don't need progressiveExplainHashKe= y. Use pid as key directly.

+ &nbs= p;         /* Update shared memo= ry with new data */
= +           strcpy(pe_= data->plan, es->str->data);
+         =   pe_data->last_print =3D GetCurrentTimestamp();

I don't think last_print is accurate because it is not= the time the execution plan
is printed but the time it was updated. I suggest last_updated_= time.

+/* Flag set by timeouts to contr= ol when to print progressive explains */
+bool       Progressi= veExplainPending =3D false;

s/print/upd= ate/

There are other point that you use= "print" but it is better to replace it with
"update".

+&nb= sp;  progressiveExplainArray =3D ShmemInitHash("progressive explain= hash",
+  = ;            = ;            = ;            = ;     50, 50,
+         &nb= sp;           &nb= sp;           &nb= sp;         &info,
+     =             =             =             =   HASH_ELEM | HASH_BLOBS);

I'm won= dering why you use "array" in the name. ProgressiveExplainHash is a
better name.

+        &= nbsp;      entry->p =3D dsa_allocate(es->= pe_a,
+  &= nbsp;           &= nbsp;           &= nbsp;            = add_size(sizeof(progressiveExplainData),
+        &n= bsp;           &n= bsp;           &n= bsp;           &n= bsp;   add_size(strlen(es->str->data),
+      =             =             =             =             =    PROGRESSIVE_EXPLAIN_ALLOC_SIZE)));

I think you need a better name for PROGRESSIVE_EXPLAIN_ALLOC_SI= ZE because it
doesn'= t reflect what it is. PROGRESSIVE_EXPLAIN_FREE_SIZE or
PROGRESSIVE_EXPLAIN_AVAIL_SIZE?

Maybe you can use dshash.

There are some comments that still refers to the wrong fu= nction name.

+/*
+ * ExecProcNodeWithExplain
+ * ExecProcNode wrapper that init= ializes progressive explain
+ * and uwraps ExecProdNode to the original function.
+ */
+static TupleTableSlot *
+ExecProcNodeExplain(PlanState *node)

and

+/*
+ * ExecProcNodeWithE= xplain
+ * Responsib= le for initialization of all structures related to progressive
+ * explains.
+ *

= +   /* state related to progressive explains */
+   struct PlanState *pe= _curr_node; 
+&= nbsp;  struct Instrumentation *pe_local_instr;
+   dsa_area   *pe_a= ;

Could you add some comments saying wh= at each of these variables are for?

I d= idn't experiment but I was wondering if there is a way to avoid the
duplicates that you adde= d to avoid the overhead.
=


--
Euler Taveira
EDB &nbs= p; https://www.enterprisedb.co= m/

--2f95292f948b4a9f804aa8253ce0ffe1--