public inbox for [email protected]
help / color / mirror / Atom feedAdd estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
50+ messages / 13 participants
[nested] [flat]
* Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
@ 2023-03-05 00:20 Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Lukas Fittl @ 2023-03-05 00:20 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>; +Cc: David Rowley <[email protected]>
Hi,
I was debugging a planner problem on Postgres 14.4 the other day - and the
involved "bad" plan was including Memoize - though I don't necessarily
think that Memoize is to blame (and this isn't any of the problems recently
fixed in Memoize costing).
However, what I noticed whilst trying different ways to fix the plan, is
that the Memoize output was a bit hard to reason about - especially since
the plan involving Memoize was expensive to run, and so I was mostly
running EXPLAIN without ANALYZE to look at the costing.
Here is an example of the output I was looking at:
-> Nested Loop (cost=1.00..971672.56 rows=119623 width=0)
-> Index Only Scan using table1_idx on table1
(cost=0.43..372676.50 rows=23553966 width=8)
-> Memoize (cost=0.57..0.61 rows=1 width=8)
Cache Key: table1.table2_id
Cache Mode: logical
-> Index Scan using table2_idx on table2
(cost=0.56..0.60 rows=1 width=8)
Index Cond: (id = table1.table2_id)
The other plan I was comparing with (that I wanted the planner to choose
instead), had a total cost of 1,451,807.35 -- and so I was trying to figure
out why the Nested Loop was costed as 971,672.56.
Simple math makes me expect the Nested Loop should roughly have a total
cost of14,740,595.76 here (372,676.50 + 23,553,966 * 0.61), ignoring a lot
of the smaller costs. Thus, in this example, it appears Memoize made the
plan cost significantly cheaper (roughly 6% of the regular cost).
Essentially this comes down to the "cost reduction" performed by Memoize
only being implicitly visible in the Nested Loop's total cost - and with
nothing useful on the Memoize node itself - since the rescan costs are not
shown.
I think explicitly adding the estimated cache hit ratio for Memoize nodes
might make this easier to reason about, like this:
-> Memoize (cost=0.57..0.61 rows=1 width=8)
Cache Key: table1.table2_id
Cache Mode: logical
Cache Hit Ratio Estimated: 0.94
Alternatively (or in addition) we could consider showing the "ndistinct"
value that is calculated in cost_memoize_rescan - since that's the most
significant contributor to the cache hit ratio (and you can influence that
directly by improving the ndistinct statistics).
See attached a patch that implements showing the cache hit ratio as a
discussion starter.
I'll park this in the July commitfest for now.
Thanks,
Lukas
--
Lukas Fittl
Attachments:
[application/octet-stream] v1-0001-Add-Estimated-Cache-Hit-Ratio-for-Memoize-plan-no.patch (5.7K, ../../CAP53Pky29GWAVVk3oBgKBDqhND0BRBN6yTPeguV_qSivFL5N_g@mail.gmail.com/3-v1-0001-Add-Estimated-Cache-Hit-Ratio-for-Memoize-plan-no.patch)
download | inline diff:
From 7eaed8f1a059bed00dfd4625931ece475b9c3451 Mon Sep 17 00:00:00 2001
From: Lukas Fittl <[email protected]>
Date: Sat, 4 Mar 2023 15:44:29 -0800
Subject: [PATCH v1] Add Estimated Cache Hit Ratio for Memoize plan nodes in
EXPLAIN
Memoize can substantially reduce the costs of certain Nested Loops, but
its hard to see this clearly, since the often significantly discounted
rescan costs are not shown in EXPLAIN.
To help understand Memoize's impact better, remember the calculated
estimated cache hit ratio, and show it in EXPLAIN output.
---
src/backend/commands/explain.c | 7 +++++++
src/backend/optimizer/path/costsize.c | 3 +++
src/backend/optimizer/plan/createplan.c | 10 +++++++---
src/backend/optimizer/util/pathnode.c | 5 +++++
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/plannodes.h | 3 +++
6 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e57bda7b62..58fda99a69 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3167,6 +3167,8 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
{
ExplainPropertyText("Cache Key", keystr.data, es);
ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+ if (es->costs)
+ ExplainPropertyFloat("Cache Hit Ratio Estimated", "", ((Memoize *) plan)->hit_ratio, 2, es);
}
else
{
@@ -3174,6 +3176,11 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
appendStringInfo(es->str, "Cache Key: %s\n", keystr.data);
ExplainIndentText(es);
appendStringInfo(es->str, "Cache Mode: %s\n", mstate->binary_mode ? "binary" : "logical");
+ if (es->costs)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Cache Hit Ratio Estimated: %0.2f\n", ((Memoize *) plan)->hit_ratio);
+ }
}
pfree(keystr.data);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 7918bb6f0d..a0fd3f8965 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2600,6 +2600,9 @@ cost_memoize_rescan(PlannerInfo *root, MemoizePath *mpath,
*/
startup_cost += cpu_tuple_cost;
+ /* Remember cache hit ratio for a potential EXPLAIN later */
+ mpath->hit_ratio = hit_ratio;
+
*rescan_startup_cost = startup_cost;
*rescan_total_cost = total_cost;
}
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index fa09a6103b..4c403cb081 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -280,7 +280,8 @@ static Material *make_material(Plan *lefttree);
static Memoize *make_memoize(Plan *lefttree, Oid *hashoperators,
Oid *collations, List *param_exprs,
bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids);
+ uint32 est_entries, Bitmapset *keyparamids,
+ double hit_ratio);
static WindowAgg *make_windowagg(List *tlist, Index winref,
int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
@@ -1691,7 +1692,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags)
plan = make_memoize(subplan, operators, collations, param_exprs,
best_path->singlerow, best_path->binary_mode,
- best_path->est_entries, keyparamids);
+ best_path->est_entries, keyparamids,
+ best_path->hit_ratio);
copy_generic_path_info(&plan->plan, (Path *) best_path);
@@ -6528,7 +6530,8 @@ materialize_finished_plan(Plan *subplan)
static Memoize *
make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
List *param_exprs, bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids)
+ uint32 est_entries, Bitmapset *keyparamids,
+ double hit_ratio)
{
Memoize *node = makeNode(Memoize);
Plan *plan = &node->plan;
@@ -6546,6 +6549,7 @@ make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
node->binary_mode = binary_mode;
node->est_entries = est_entries;
node->keyparamids = keyparamids;
+ node->hit_ratio = hit_ratio;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index d749b50578..d4186784c5 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1624,6 +1624,11 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
*/
pathnode->est_entries = 0;
+ /*
+ * The estimated cache hit ratio will calculated later by cost_memoize_rescan()
+ */
+ pathnode->hit_ratio = 0;
+
/*
* Add a small additional charge for caching the first entry. All the
* harder calculations for rescans are performed in cost_memoize_rescan().
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index d61a62da19..1b7cab3bc5 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1962,6 +1962,7 @@ typedef struct MemoizePath
uint32 est_entries; /* The maximum number of entries that the
* planner expects will fit in the cache, or 0
* if unknown */
+ double hit_ratio; /* Estimated cache hit ratio, kept for EXPLAIN */
} MemoizePath;
/*
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 659bd05c0c..7966062a32 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -923,6 +923,9 @@ typedef struct Memoize
/* paramids from param_exprs */
Bitmapset *keyparamids;
+
+ /* Estimated cache hit ratio, kept for EXPLAIN */
+ double hit_ratio;
} Memoize;
/* ----------------
--
2.34.0
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
@ 2023-03-07 09:51 ` David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: David Rowley @ 2023-03-07 09:51 UTC (permalink / raw)
To: Lukas Fittl <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Sun, 5 Mar 2023 at 13:21, Lukas Fittl <[email protected]> wrote:
> Alternatively (or in addition) we could consider showing the "ndistinct" value that is calculated in cost_memoize_rescan - since that's the most significant contributor to the cache hit ratio (and you can influence that directly by improving the ndistinct statistics).
I think the ndistinct estimate plus the est_entries together would be
useful. I think showing just the hit ratio number might often just
raise too many questions about how that's calculated. To calculate the
hit ratio we need to estimate the number of entries that can be kept
in the cache at once and also the number of input rows and the number
of distinct values. We can see the input rows by looking at the outer
side of the join in EXPLAIN, but we've no idea about the ndistinct or
how many items the planner thought could be kept in the cache at once.
The plan node already has est_entries, so it should just be a matter
of storing the ndistinct estimate in the Path and putting it into the
Plan node so the executor has access to it during EXPLAIN.
David
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2023-07-06 07:56 ` Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Daniel Gustafsson @ 2023-07-06 07:56 UTC (permalink / raw)
To: Lukas Fittl <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Rowley <[email protected]>
> On 7 Mar 2023, at 10:51, David Rowley <[email protected]> wrote:
>
> On Sun, 5 Mar 2023 at 13:21, Lukas Fittl <[email protected]> wrote:
>> Alternatively (or in addition) we could consider showing the "ndistinct" value that is calculated in cost_memoize_rescan - since that's the most significant contributor to the cache hit ratio (and you can influence that directly by improving the ndistinct statistics).
>
> I think the ndistinct estimate plus the est_entries together would be
> useful. I think showing just the hit ratio number might often just
> raise too many questions about how that's calculated. To calculate the
> hit ratio we need to estimate the number of entries that can be kept
> in the cache at once and also the number of input rows and the number
> of distinct values. We can see the input rows by looking at the outer
> side of the join in EXPLAIN, but we've no idea about the ndistinct or
> how many items the planner thought could be kept in the cache at once.
>
> The plan node already has est_entries, so it should just be a matter
> of storing the ndistinct estimate in the Path and putting it into the
> Plan node so the executor has access to it during EXPLAIN.
Lukas: do you have an updated patch for this commitfest to address David's
comments?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
@ 2023-07-06 08:27 ` Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Lukas Fittl @ 2023-07-06 08:27 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Rowley <[email protected]>
On Thu, Jul 6, 2023 at 12:56 AM Daniel Gustafsson <[email protected]> wrote:
> Lukas: do you have an updated patch for this commitfest to address David's
> comments?
>
I have a draft - I should be able to post an updated patch in the next
days. Thanks for checking!
Thanks,
Lukas
--
Lukas Fittl
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
@ 2025-03-20 08:47 ` Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-20 08:47 UTC (permalink / raw)
To: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; David Rowley <[email protected]>
On 06.07.2023 11:27, Lukas Fittl wrote:
> On Thu, Jul 6, 2023 at 12:56 AM Daniel Gustafsson <[email protected]> wrote:
>
> Lukas: do you have an updated patch for this commitfest to address
> David's
> comments?
>
>
> I have a draft - I should be able to post an updated patch in the next
> days. Thanks for checking!
>
> Thanks,
> Lukas
>
> --
> Lukas Fittl
Hi hackers,
While debugging a query execution plan involving Memoize, it'd be nice
to determine how many unique keys would fit into the cache. The
est_entries value provides some insight, but without knowing ndistinct,
it is unclear whether the cache is large enough to hold all unique keys
or if some will be evicted.
Given its potential usefulness, I would like to work for this. I
attached v2 patch with changes.
Example from memoize.sql
EXPLAIN SELECT COUNT(*),AVG(t1.unique1) FROM tenk1 t1
INNER JOIN tenk1 t2 ON t1.unique1 = t2.thousand
WHERE t2.unique1 < 1200;
QUERY PLAN
-----------------------------------------------------------------------------------------------------
Aggregate (cost=815.12..815.13 rows=1 width=40)
-> Nested Loop (cost=0.30..809.12 rows=1200 width=4)
-> Seq Scan on tenk1 t2 (cost=0.00..470.00 rows=1200 width=4)
Filter: (unique1 < 1200)
-> Memoize (cost=0.30..0.41 rows=1 width=4)
Cache Key: t2.thousand
Cache Mode: logical
Cache Estimated Entries: 655
Cache Estimated NDistinct: 721
-> Index Only Scan using tenk1_unique1 on tenk1 t1
(cost=0.29..0.40 rows=1 width=4)
Index Cond: (unique1 = t2.thousand)
(11 rows)
Additionally, since this information would only be shown in EXPLAIN when
costs are enabled, it should not cause any performance regression in
normal execution. However, reviewers should be especially careful when
verifying test outputs, as this change could affect plan details in
regression tests.
Any thoughts?
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v2-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch (5.9K, ../../[email protected]/3-v2-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch)
download | inline diff:
From 2e7d9bd9bfe58825fbdbb0100b9bd19d5bb7e53b Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Thu, 20 Mar 2025 11:45:02 +0300
Subject: [PATCH v2] Show ndistinct and est_entries in EXPLAIN for Memoize
---
src/backend/commands/explain.c | 12 ++++++++++++
src/backend/optimizer/path/costsize.c | 3 +++
src/backend/optimizer/plan/createplan.c | 10 +++++++---
src/backend/optimizer/util/pathnode.c | 3 +++
src/include/nodes/pathnodes.h | 2 ++
src/include/nodes/plannodes.h | 6 ++++++
6 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 33a16d2d8e2..95110e63c7d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3629,6 +3629,11 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
{
ExplainPropertyText("Cache Key", keystr.data, es);
ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+ if (es->costs)
+ {
+ ExplainPropertyFloat("Cache Estimated Entries", "", ((Memoize *) plan)->est_entries, 0, es);
+ ExplainPropertyFloat("Cache Estimated NDistinct", "", ((Memoize *) plan)->ndistinct, 0, es);
+ }
}
else
{
@@ -3636,6 +3641,13 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
appendStringInfo(es->str, "Cache Key: %s\n", keystr.data);
ExplainIndentText(es);
appendStringInfo(es->str, "Cache Mode: %s\n", mstate->binary_mode ? "binary" : "logical");
+ if (es->costs)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Cache Estimated Entries: %d\n", ((Memoize *) plan)->est_entries);
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Cache Estimated NDistinct: %0.0f\n", ((Memoize *) plan)->ndistinct);
+ }
}
pfree(keystr.data);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 256568d05a2..9c0738da4fe 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2604,6 +2604,9 @@ cost_memoize_rescan(PlannerInfo *root, MemoizePath *mpath,
mpath->est_entries = Min(Min(ndistinct, est_cache_entries),
PG_UINT32_MAX);
+ /* Remember ndistinct for a potential EXPLAIN later */
+ mpath->ndistinct = ndistinct;
+
/*
* When the number of distinct parameter values is above the amount we can
* store in the cache, then we'll have to evict some entries from the
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 75e2b0b9036..79795a2b5f2 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -284,7 +284,8 @@ static Material *make_material(Plan *lefttree);
static Memoize *make_memoize(Plan *lefttree, Oid *hashoperators,
Oid *collations, List *param_exprs,
bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids);
+ uint32 est_entries, Bitmapset *keyparamids,
+ double ndistinct);
static WindowAgg *make_windowagg(List *tlist, WindowClause *wc,
int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
@@ -1703,7 +1704,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags)
plan = make_memoize(subplan, operators, collations, param_exprs,
best_path->singlerow, best_path->binary_mode,
- best_path->est_entries, keyparamids);
+ best_path->est_entries, keyparamids,
+ best_path->ndistinct);
copy_generic_path_info(&plan->plan, (Path *) best_path);
@@ -6636,7 +6638,8 @@ materialize_finished_plan(Plan *subplan)
static Memoize *
make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
List *param_exprs, bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids)
+ uint32 est_entries, Bitmapset *keyparamids,
+ double ndistinct)
{
Memoize *node = makeNode(Memoize);
Plan *plan = &node->plan;
@@ -6654,6 +6657,7 @@ make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
node->binary_mode = binary_mode;
node->est_entries = est_entries;
node->keyparamids = keyparamids;
+ node->ndistinct = ndistinct;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..1676e20879a 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1701,6 +1701,9 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
Assert(enable_memoize);
pathnode->path.disabled_nodes = subpath->disabled_nodes;
+ /* Estimated number of distinct memoization keys, computed using estimate_num_groups() */
+ pathnode->ndistinct = 0;
+
/*
* Add a small additional charge for caching the first entry. All the
* harder calculations for rescans are performed in cost_memoize_rescan().
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c24a1fc8514..e99dbee2b3f 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2040,6 +2040,8 @@ typedef struct MemoizePath
uint32 est_entries; /* The maximum number of entries that the
* planner expects will fit in the cache, or 0
* if unknown */
+ double ndistinct; /* Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN */
} MemoizePath;
/*
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index f78bffd90cf..16b307a2141 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1060,6 +1060,12 @@ typedef struct Memoize
/* paramids from param_exprs */
Bitmapset *keyparamids;
+
+ /*
+ * Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN
+ */
+ double ndistinct;
} Memoize;
/* ----------------
--
2.34.1
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
@ 2025-03-20 10:37 ` David Rowley <[email protected]>
2025-03-20 12:04 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: David Rowley @ 2025-03-20 10:37 UTC (permalink / raw)
To: Ilia Evdokimov <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 20 Mar 2025 at 21:48, Ilia Evdokimov
<[email protected]> wrote:
> -> Memoize (cost=0.30..0.41 rows=1 width=4)
> Cache Key: t2.thousand
> Cache Mode: logical
> Cache Estimated Entries: 655
> Cache Estimated NDistinct: 721
> -> Index Only Scan using tenk1_unique1 on tenk1 t1 (cost=0.29..0.40 rows=1 width=4)
> Index Cond: (unique1 = t2.thousand)
> (11 rows)
>
> Additionally, since this information would only be shown in EXPLAIN when costs are enabled, it should not cause any performance regression in normal execution. However, reviewers should be especially careful when verifying test outputs, as this change could affect plan details in regression tests.
>
> Any thoughts?
> + ExplainIndentText(es);
> + appendStringInfo(es->str, "Cache Estimated Entries: %d\n", ((Memoize *) plan)->est_entries);
> + ExplainIndentText(es);
> + appendStringInfo(es->str, "Cache Estimated NDistinct: %0.0f\n", ((Memoize *) plan)->ndistinct);
est_entries is a uint32, so %u is the correct format character for that type.
I don't think you need to prefix all these properties with "Cache"
just because the other two properties have that prefix. I also don't
think the names you've chosen really reflect the meaning. How about
something like: "Estimated Distinct Lookup Keys: 721 Estimated
Capacity: 655", in that order. I think maybe having that as one line
for format=text is better than 2 lines. The EXPLAIN output is already
often taking up more lines in v18 than in v17, would be good to not
make that even worse unnecessarily.
I see the existing code there could use ExplainPropertyText rather
than have a special case for text and non-text formats. That's likely
my fault. If we're touching this code, then we should probably tidy
that up. Do you want to create a precursor fixup patch for that?
+ double ndistinct; /* Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN */
Maybe this field in MemoizePath needs a better name. How about
"est_unique_keys"? and also do the rename in struct Memoize.
I'm also slightly concerned about making struct Memoize bigger. I had
issues with a performance regression [1] for 908a96861 when increasing
the WindowAgg struct size last year and the only way I found to make
it go away was to shuffle the fields around so that the struct size
didn't increase. I think we'll need to see a benchmark of a query that
hits Memoize quite hard with a small cache size to see if the
performance decreases as a result of adding the ndistinct field. It's
unfortunate that we'll not have the luxury of squeezing this double
into padding if we do see a slowdown.
David
[1] https://postgr.es/m/flat/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-20 12:04 ` Ilia Evdokimov <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-20 12:04 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On 20.03.2025 13:37, David Rowley wrote:
> est_entries is a uint32, so %u is the correct format character for that type.
>
> I don't think you need to prefix all these properties with "Cache"
> just because the other two properties have that prefix. I also don't
> think the names you've chosen really reflect the meaning. How about
> something like: "Estimated Distinct Lookup Keys: 721 Estimated
> Capacity: 655", in that order. I think maybe having that as one line
> for format=text is better than 2 lines. The EXPLAIN output is already
> often taking up more lines in v18 than in v17, would be good to not
> make that even worse unnecessarily.
LGTM
> I see the existing code there could use ExplainPropertyText rather
> than have a special case for text and non-text formats. That's likely
> my fault. If we're touching this code, then we should probably tidy
> that up. Do you want to create a precursor fixup patch for that?
I fully agree with this suggestion. Then I'll begin with this on another
new thread.
> + double ndistinct; /* Estimated number of distinct memoization keys,
> + * used for cache size evaluation. Kept for EXPLAIN */
>
> Maybe this field in MemoizePath needs a better name. How about
> "est_unique_keys"? and also do the rename in struct Memoize.
LGTM
> I'm also slightly concerned about making struct Memoize bigger. I had
> issues with a performance regression [1] for 908a96861 when increasing
> the WindowAgg struct size last year and the only way I found to make
> it go away was to shuffle the fields around so that the struct size
> didn't increase. I think we'll need to see a benchmark of a query that
> hits Memoize quite hard with a small cache size to see if the
> performance decreases as a result of adding the ndistinct field. It's
> unfortunate that we'll not have the luxury of squeezing this double
> into padding if we do see a slowdown.
I tried rearranging the fields in the structure with 'ndistinct' field,
but unfortunately, sizeof(Memoize) did not remain the same. Therefore,
benchmarking Memoize is definitely necessary. Thanks for the information!
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-20 12:32 ` Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-20 12:32 UTC (permalink / raw)
To: David Rowley <[email protected]>; Ilia Evdokimov <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On 20/3/2025 11:37, David Rowley wrote:
> I'm also slightly concerned about making struct Memoize bigger. I had
> issues with a performance regression [1] for 908a96861 when increasing
> the WindowAgg struct size last year and the only way I found to make
> it go away was to shuffle the fields around so that the struct size
> didn't increase. I think we'll need to see a benchmark of a query that
> hits Memoize quite hard with a small cache size to see if the
> performance decreases as a result of adding the ndistinct field. It's
> unfortunate that we'll not have the luxury of squeezing this double
> into padding if we do see a slowdown.
I quite frequently need the number of distinct values (or groups)
predicted during the Memoize node creation to understand why caching is
sometimes employed or not.
But I had thought about an alternative way: having an extensible EXPLAIN
(thanks to Robert), we may save optimisation-stage data (I have the same
necessity in the case of IncrementalSort, for example) and put it into
the Plan node on-demand. So, the way I want to go is a Plan::extlist
node and create_plan hook, which may allow copying best_path data to the
final plan. So, here, we will add a new parameter and avoid touching the
core code.
But I would give +1 to current approach if it were done in a shorter time.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-20 14:03 ` Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-20 14:03 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; David Rowley <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On 20.03.2025 15:32, Andrei Lepikhov wrote:
> I quite frequently need the number of distinct values (or groups)
> predicted during the Memoize node creation to understand why caching
> is sometimes employed or not.
> But I had thought about an alternative way: having an extensible
> EXPLAIN (thanks to Robert), we may save optimisation-stage data (I
> have the same necessity in the case of IncrementalSort, for example)
> and put it into the Plan node on-demand. So, the way I want to go is a
> Plan::extlist node and create_plan hook, which may allow copying
> best_path data to the final plan. So, here, we will add a new
> parameter and avoid touching the core code.
> But I would give +1 to current approach if it were done in a shorter
> time.
Then before proceeding further, I think we need to benchmark this change
to ensure there are no performance regressions. If performance issues
arise, then using extensible EXPLAIN might be the only viable approach.
I have addressed most of the review comments except for the
ExplainPropertyText change. I am attaching v3 of the patch with these
updates. If anyone notices any performance issues, please let me know.
Issue with ExplainPropertyText for this thread I'll fix in the next
patches if it will be necessary.
So far, I have tested it on small machines. There are no performance
issues yet. I'll run benchmarks on larger ones soon.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v3-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch (5.9K, ../../[email protected]/2-v3-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch)
download | inline diff:
From 02a9b4793deb74f2e67dacabec259c4cd929d995 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Thu, 20 Mar 2025 16:58:48 +0300
Subject: [PATCH v3] Show ndistinct and est_entries in EXPLAIN for Memoize
---
src/backend/commands/explain.c | 12 ++++++++++++
src/backend/optimizer/path/costsize.c | 3 +++
src/backend/optimizer/plan/createplan.c | 10 +++++++---
src/backend/optimizer/util/pathnode.c | 3 +++
src/include/nodes/pathnodes.h | 2 ++
src/include/nodes/plannodes.h | 6 ++++++
6 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 33a16d2d8e2..4d8e94d3feb 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3629,6 +3629,11 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
{
ExplainPropertyText("Cache Key", keystr.data, es);
ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+ if (es->costs)
+ {
+ ExplainPropertyFloat("Estimated Capacity", "", ((Memoize *) plan)->est_entries, 0, es);
+ ExplainPropertyFloat("Estimated Distinct Lookup Keys", "", ((Memoize *) plan)->est_unique_keys, 0, es);
+ }
}
else
{
@@ -3636,6 +3641,13 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
appendStringInfo(es->str, "Cache Key: %s\n", keystr.data);
ExplainIndentText(es);
appendStringInfo(es->str, "Cache Mode: %s\n", mstate->binary_mode ? "binary" : "logical");
+ if (es->costs)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Estimated Capacity: %u, Estimated Distinct Lookup Keys: %0.0f\n",
+ ((Memoize *) plan)->est_entries,
+ ((Memoize *) plan)->est_unique_keys);
+ }
}
pfree(keystr.data);
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 256568d05a2..4e42afbf53a 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2604,6 +2604,9 @@ cost_memoize_rescan(PlannerInfo *root, MemoizePath *mpath,
mpath->est_entries = Min(Min(ndistinct, est_cache_entries),
PG_UINT32_MAX);
+ /* Remember ndistinct for a potential EXPLAIN later */
+ mpath->est_unique_keys = ndistinct;
+
/*
* When the number of distinct parameter values is above the amount we can
* store in the cache, then we'll have to evict some entries from the
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 75e2b0b9036..596b82c5dc9 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -284,7 +284,8 @@ static Material *make_material(Plan *lefttree);
static Memoize *make_memoize(Plan *lefttree, Oid *hashoperators,
Oid *collations, List *param_exprs,
bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids);
+ uint32 est_entries, Bitmapset *keyparamids,
+ double est_unique_keys);
static WindowAgg *make_windowagg(List *tlist, WindowClause *wc,
int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
@@ -1703,7 +1704,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags)
plan = make_memoize(subplan, operators, collations, param_exprs,
best_path->singlerow, best_path->binary_mode,
- best_path->est_entries, keyparamids);
+ best_path->est_entries, keyparamids,
+ best_path->est_unique_keys);
copy_generic_path_info(&plan->plan, (Path *) best_path);
@@ -6636,7 +6638,8 @@ materialize_finished_plan(Plan *subplan)
static Memoize *
make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
List *param_exprs, bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids)
+ uint32 est_entries, Bitmapset *keyparamids,
+ double est_unique_keys)
{
Memoize *node = makeNode(Memoize);
Plan *plan = &node->plan;
@@ -6654,6 +6657,7 @@ make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
node->binary_mode = binary_mode;
node->est_entries = est_entries;
node->keyparamids = keyparamids;
+ node->est_unique_keys = est_unique_keys;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..1fbcda99067 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1701,6 +1701,9 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
Assert(enable_memoize);
pathnode->path.disabled_nodes = subpath->disabled_nodes;
+ /* Estimated number of distinct memoization keys, computed using estimate_num_groups() */
+ pathnode->est_unique_keys = 0;
+
/*
* Add a small additional charge for caching the first entry. All the
* harder calculations for rescans are performed in cost_memoize_rescan().
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c24a1fc8514..0946ccea994 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2040,6 +2040,8 @@ typedef struct MemoizePath
uint32 est_entries; /* The maximum number of entries that the
* planner expects will fit in the cache, or 0
* if unknown */
+ double est_unique_keys; /* Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN */
} MemoizePath;
/*
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index f78bffd90cf..b38d4d91fb4 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1060,6 +1060,12 @@ typedef struct Memoize
/* paramids from param_exprs */
Bitmapset *keyparamids;
+
+ /*
+ * Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN
+ */
+ double est_unique_keys;
} Memoize;
/* ----------------
--
2.34.1
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
@ 2025-03-20 18:54 ` Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-20 18:54 UTC (permalink / raw)
To: Ilia Evdokimov <[email protected]>; David Rowley <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On 20/3/2025 15:03, Ilia Evdokimov wrote:
>
> On 20.03.2025 15:32, Andrei Lepikhov wrote:
>> I quite frequently need the number of distinct values (or groups)
>> predicted during the Memoize node creation to understand why caching
>> is sometimes employed or not.
>> But I had thought about an alternative way: having an extensible
>> EXPLAIN (thanks to Robert), we may save optimisation-stage data (I
>> have the same necessity in the case of IncrementalSort, for example)
>> and put it into the Plan node on-demand. So, the way I want to go is a
>> Plan::extlist node and create_plan hook, which may allow copying
>> best_path data to the final plan. So, here, we will add a new
>> parameter and avoid touching the core code.
>> But I would give +1 to current approach if it were done in a shorter
>> time.
>
>
> Then before proceeding further, I think we need to benchmark this change
> to ensure there are no performance regressions. If performance issues
> arise, then using extensible EXPLAIN might be the only viable approach.
>
> I have addressed most of the review comments except for the
> ExplainPropertyText change. I am attaching v3 of the patch with these
> updates. If anyone notices any performance issues, please let me know.
> Issue with ExplainPropertyText for this thread I'll fix in the next
> patches if it will be necessary.
>
> So far, I have tested it on small machines. There are no performance
> issues yet. I'll run benchmarks on larger ones soon.
I have some doubts here.
The number of distinct values says something only when it has taken
together with the number of calls.
Frequently, one of the caching keys is from outer table A (10 tuples),
and another is from outer table B (100 tuples). Calculating the number
of successful cache fetches predicted by the planner may not be evident
in the case of a composite cache key.
What I may propose here is:
1. Use fraction of calls, for example - 50% duplicated key values.
2. Show the calculated hit and eviction ratio.
The second option looks better to me. It is pretty understandable by a
user and may be compared to the numbers obtained during the execution.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-21 02:50 ` David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: David Rowley @ 2025-03-21 02:50 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 21 Mar 2025 at 07:54, Andrei Lepikhov <[email protected]> wrote:
> I have some doubts here.
> The number of distinct values says something only when it has taken
> together with the number of calls.
Couldn't the reader just look at the Nested Loop's outer side row
estimate for that?
> Frequently, one of the caching keys is from outer table A (10 tuples),
> and another is from outer table B (100 tuples). Calculating the number
> of successful cache fetches predicted by the planner may not be evident
> in the case of a composite cache key.
>
> What I may propose here is:
> 1. Use fraction of calls, for example - 50% duplicated key values.
> 2. Show the calculated hit and eviction ratio.
My concern with showing just the estimated hit and evict ratios it
that it might lead to more questions, like how those were calculated.
However, I can see the argument for having one or both of these in
addition to the expected unique keys and expected cache capacity.
I think the primary factors in how useful Memoize is are: 1) How many
items do we expect to be able to store in the cache concurrently, and;
2) How many unique lookups keys do we expect to be looked up, and; 3)
The total number of expected lookups. #1 is quite difficult to
figure out (maybe by looking at row width and row estimates) and
there's just no information about #2. #3 is already shown, in the
Nested Loop's outer side.
The reason that the hit and evict ratios might also be useful are that
it you need a bit of inside knowledge on how the 3 input values are
used to calculate these. For example you might come up with a higher
estimated hit ratio if you assumed the keys arrived in order vs
unordered. If they're unordered and you don't have room for all keys
in the cache at once then that increases the chances that Memoize had
to evict something that will be needed for a future lookup.
Also, I was just looking back at the concern I had with increasing the
size of struct Memoize. I suspect that might not be that much of a
concern. The WindowAgg problem I mentioned was with the executor
state, not the plan node. I see the only time we access the plan node
for Memoize during execution is when we call build_hash_table().
David
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-21 09:02 ` Andrei Lepikhov <[email protected]>
2025-03-21 10:08 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-21 09:02 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On 21/3/2025 03:50, David Rowley wrote:
> On Fri, 21 Mar 2025 at 07:54, Andrei Lepikhov <[email protected]> wrote:
>> I have some doubts here.
>> The number of distinct values says something only when it has taken
>> together with the number of calls.
>
> Couldn't the reader just look at the Nested Loop's outer side row
> estimate for that?
In my cases, key sources are usually spread across dozens of joins, and
it is visually hard to find out (especially when we have an EXPLAIN
ANALYSE VERBOSE) the JOIN operator to extract the number of calls. The
hit ratio, meanwhile, may be analysed locally in the Memoize node. For
example, 80% (0.8) is evidently a good one, 40% is questionable, and 5%
is too low and we should avoid Memoize here.
May it be beaten by just printing the "calls" number at the Memoize output?
>
>> Frequently, one of the caching keys is from outer table A (10 tuples),
>> and another is from outer table B (100 tuples). Calculating the number
>> of successful cache fetches predicted by the planner may not be evident
>> in the case of a composite cache key.
>>
>> What I may propose here is:
>> 1. Use fraction of calls, for example - 50% duplicated key values.
>> 2. Show the calculated hit and eviction ratio.
>
> I think the primary factors in how useful Memoize is are: 1) How many
> items do we expect to be able to store in the cache concurrently, and;
> 2) How many unique lookups keys do we expect to be looked up, and; 3)
> The total number of expected lookups. #1 is quite difficult to
> figure out (maybe by looking at row width and row estimates) and
> there's just no information about #2. #3 is already shown, in the
> Nested Loop's outer side.
It depends on the task. If you are looking for the answer to how precise
the group's estimation has been (to check statistics), I agree. In cases
I have seen before, the main question is how effective was (or maybe) a
Memoize node == how often the incoming key fits the cache. In that case,
the hit ratio fraction is more understandable for a broad audience.
That's why according to my experience in case of a good cache
reusability factor, users are usually okay with increasing the cache
size to the necessary numbers and avoiding evictions at all costs. So,
the predicted evict_ratio also tells us about incrementing work_mem to
enhance the chances of Memoisation.
Having written the last sentence I came back to the point why work_mem
is so universal and is used at each node as a criteria of memory
allocation size? But it is a different story, I think.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-21 10:08 ` Ilia Evdokimov <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-21 10:08 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; David Rowley <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
After considering the opinions expressed in this discussion, I tend to
agree more with David. If we add info about estimated unique keys and
estimated capacity, then any additional information - such as
evict_ratio and hit_ratio - can also be calculated, as EXPLAIN ANALYZE
provides all the necessary details to compute these values.
For now, I’m attaching a rebased v4 patch, which includes the fix for
ExplainPropertyText.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v4-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch (5.7K, ../../[email protected]/2-v4-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch)
download | inline diff:
From b60e508583b1cb7f30814cef6f39c4b570693350 Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <[email protected]>
Date: Fri, 21 Mar 2025 11:52:29 +0300
Subject: [PATCH v4] Show ndistinct and est_entries in EXPLAIN for Memoize
---
src/backend/commands/explain.c | 16 ++++++++++++++++
src/backend/optimizer/path/costsize.c | 3 +++
src/backend/optimizer/plan/createplan.c | 10 +++++++---
src/backend/optimizer/util/pathnode.c | 3 +++
src/include/nodes/pathnodes.h | 2 ++
src/include/nodes/plannodes.h | 6 ++++++
6 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 391b34a2af2..cb7e295ca8a 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3628,6 +3628,22 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
ExplainPropertyText("Cache Key", keystr.data, es);
ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+ if (es->costs)
+ {
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Estimated Capacity: %u Estimated Distinct Lookup Keys: %0.0f\n",
+ ((Memoize *) plan)->est_entries,
+ ((Memoize *) plan)->est_unique_keys);
+ }
+ else
+ {
+ ExplainPropertyUInteger("Estimated Capacity", "", ((Memoize *) plan)->est_entries, es);
+ ExplainPropertyFloat("Estimated Distinct Lookup Keys", "", ((Memoize *) plan)->est_unique_keys, 0, es);
+ }
+ }
+
pfree(keystr.data);
if (!es->analyze)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index f6f77b8fe19..e3abf9ccc26 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2604,6 +2604,9 @@ cost_memoize_rescan(PlannerInfo *root, MemoizePath *mpath,
mpath->est_entries = Min(Min(ndistinct, est_cache_entries),
PG_UINT32_MAX);
+ /* Remember ndistinct for a potential EXPLAIN later */
+ mpath->est_unique_keys = ndistinct;
+
/*
* When the number of distinct parameter values is above the amount we can
* store in the cache, then we'll have to evict some entries from the
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 75e2b0b9036..596b82c5dc9 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -284,7 +284,8 @@ static Material *make_material(Plan *lefttree);
static Memoize *make_memoize(Plan *lefttree, Oid *hashoperators,
Oid *collations, List *param_exprs,
bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids);
+ uint32 est_entries, Bitmapset *keyparamids,
+ double est_unique_keys);
static WindowAgg *make_windowagg(List *tlist, WindowClause *wc,
int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
@@ -1703,7 +1704,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags)
plan = make_memoize(subplan, operators, collations, param_exprs,
best_path->singlerow, best_path->binary_mode,
- best_path->est_entries, keyparamids);
+ best_path->est_entries, keyparamids,
+ best_path->est_unique_keys);
copy_generic_path_info(&plan->plan, (Path *) best_path);
@@ -6636,7 +6638,8 @@ materialize_finished_plan(Plan *subplan)
static Memoize *
make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
List *param_exprs, bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids)
+ uint32 est_entries, Bitmapset *keyparamids,
+ double est_unique_keys)
{
Memoize *node = makeNode(Memoize);
Plan *plan = &node->plan;
@@ -6654,6 +6657,7 @@ make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
node->binary_mode = binary_mode;
node->est_entries = est_entries;
node->keyparamids = keyparamids;
+ node->est_unique_keys = est_unique_keys;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..1fbcda99067 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1701,6 +1701,9 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
Assert(enable_memoize);
pathnode->path.disabled_nodes = subpath->disabled_nodes;
+ /* Estimated number of distinct memoization keys, computed using estimate_num_groups() */
+ pathnode->est_unique_keys = 0;
+
/*
* Add a small additional charge for caching the first entry. All the
* harder calculations for rescans are performed in cost_memoize_rescan().
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c24a1fc8514..0946ccea994 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2040,6 +2040,8 @@ typedef struct MemoizePath
uint32 est_entries; /* The maximum number of entries that the
* planner expects will fit in the cache, or 0
* if unknown */
+ double est_unique_keys; /* Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN */
} MemoizePath;
/*
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index f78bffd90cf..b38d4d91fb4 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1060,6 +1060,12 @@ typedef struct Memoize
/* paramids from param_exprs */
Bitmapset *keyparamids;
+
+ /*
+ * Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN
+ */
+ double est_unique_keys;
} Memoize;
/* ----------------
--
2.34.1
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-23 21:16 ` David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: David Rowley @ 2025-03-23 21:16 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 21 Mar 2025 at 22:02, Andrei Lepikhov <[email protected]> wrote:
> In cases
> I have seen before, the main question is how effective was (or maybe) a
> Memoize node == how often the incoming key fits the cache. In that case,
> the hit ratio fraction is more understandable for a broad audience.
> That's why according to my experience in case of a good cache
> reusability factor, users are usually okay with increasing the cache
> size to the necessary numbers and avoiding evictions at all costs. So,
> the predicted evict_ratio also tells us about incrementing work_mem to
> enhance the chances of Memoisation.
Can you explain why "Estimated Capacity" and "Estimated Distinct
Lookup Keys" don't answer that? If there are more distinct lookup
keys than there is capacity to store them, then some will be evicted.
Once again, I'm not necessarily objecting to hit and evict ratios
being shown, I just want to know they're actually useful enough to
show and don't just bloat the EXPLAIN output needlessly. So far your
arguments aren't convincing me that they are.
> Having written the last sentence I came back to the point why work_mem
> is so universal and is used at each node as a criteria of memory
> allocation size? But it is a different story, I think.
We have to set the limit somehow. We could have done this by having a
GUC per node type that uses memory, but it looks like something more
universal was decided, perhaps to save on GUCs. I don't know the exact
history, but once upon a time, sort_mem existed. Perhaps that
disappeared because we grew more node types that needed to allocate
large, otherwise unbounded amounts of memory. We did more recently
grow a hash_mem_multiplier GUC, so it's not true to say that work_mem
solely controls the limits of each node's memory allocation sizes.
David
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-24 21:23 ` Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-24 21:23 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On 3/23/25 22:16, David Rowley wrote:
> On Fri, 21 Mar 2025 at 22:02, Andrei Lepikhov <[email protected]> wrote:
> Can you explain why "Estimated Capacity" and "Estimated Distinct
> Lookup Keys" don't answer that? If there are more distinct lookup
> keys than there is capacity to store them, then some will be evicted.
I wouldn't say these parameters don't answer. I try to debate how usable
they are. To be more practical, let me demonstrate the example:
EXPLAIN (COSTS OFF) SELECT * FROM t1,t2 WHERE t1.x=t2.x;
Nested Loop (cost=0.44..7312.65 rows=211330 width=33)
-> Seq Scan on t1 (cost=0.00..492.00 rows=30000 width=22)
-> Memoize (cost=0.44..3.82 rows=7 width=11)
Cache Key: t1.x
Cache Mode: logical
Estimated Capacity: 1001 Estimated Distinct Lookup Keys: 1001
-> Index Scan using t2_x_idx2 on t2 (cost=0.43..3.81 rows=7)
Index Cond: (x = t1.x)
At first, I began to look for documentation because it was unclear what
both new parameters specifically meant. Okay, there was no documentation
but trivial code, and after a short discovery, I realised the meaning.
The first fact I see from this EXPLAIN is that Postgres estimates it has
enough memory to fit all the entries. Okay, but what does it give me? I
may just increase work_mem and provide the query with more memory if
needed. My main concern is how frequently this cache is planned to be
used. Doing some mental effort, I found the line "rows=30000."
Calculating a bit more, I may suppose it means that we have a 95% chance
to reuse the cache. Okay, I got it.
Now, see:
1. I needed to discover the meaning of the new parameters because they
were different from the earlier "hit" and "miss."
2. I need to find a common JOIN for keys of this node. Imagine a typical
200-row EXPLAIN with 2-3 Memoization keys from different tables.
3. I need to make calculations
On the opposite, the hit ratio, written instead, already known by
analogy, already provides me with necessary cache efficacy data; no need
to watch outside the node; it may be easily compared with the actual
value. Am I wrong?
Both approaches provide the data, but each one is more usable?
I think we may ask more people, for example, Nikolay Samokhvalov, who,
as I heard, works hard with explains.
>
> Once again, I'm not necessarily objecting to hit and evict ratios
> being shown, I just want to know they're actually useful enough to
> show and don't just bloat the EXPLAIN output needlessly. So far your
> arguments aren't convincing me that they are.
I'm -1 for this redundancy.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-24 22:05 ` David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-25 06:40 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: David Rowley @ 2025-03-24 22:05 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On Tue, 25 Mar 2025 at 10:23, Andrei Lepikhov <[email protected]> wrote:
>
> On 3/23/25 22:16, David Rowley wrote:
> > Once again, I'm not necessarily objecting to hit and evict ratios
> > being shown, I just want to know they're actually useful enough to
> > show and don't just bloat the EXPLAIN output needlessly. So far your
> > arguments aren't convincing me that they are.
> I'm -1 for this redundancy.
I'm not following what the -1 is for. Is it for showing both hit and
evict ratios? And your vote is only for adding hit ratio?
Just to make it clear, the evict ratio isn't redundant because we show
hit ratio. If you have 1000 calls and 1000 distinct values and enough
memory to store those, then that's a 0% hit ratio since the first
lookup is a miss. If you change the calls to 2000 then that's a 50%
hit ratio and still 0% evict.
David
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-24 22:15 ` Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-25 03:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
1 sibling, 2 replies; 50+ messages in thread
From: Tom Lane @ 2025-03-24 22:15 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
David Rowley <[email protected]> writes:
> I'm not following what the -1 is for. Is it for showing both hit and
> evict ratios? And your vote is only for adding hit ratio?
> Just to make it clear, the evict ratio isn't redundant because we show
> hit ratio. If you have 1000 calls and 1000 distinct values and enough
> memory to store those, then that's a 0% hit ratio since the first
> lookup is a miss. If you change the calls to 2000 then that's a 50%
> hit ratio and still 0% evict.
FWIW, I share these doubts about whether these values are useful
enough to include in the default EXPLAIN output. My main beef
with them though is that they are basically numbers derived along
the way to producing a cost estimate, and I don't think we break
out such intermediate results for other node types.
It's looking like Robert's "pg_overexplain" will hit the tree soon,
so maybe there could be a case for teaching that to emit additional
costing details such as these? I'd kind of like to see a larger
scope than just Memoize for such an addition, though I'm not sure
exactly which other intermediate estimates are worth showing.
regards, tom lane
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
@ 2025-03-24 22:30 ` Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Lukas Fittl @ 2025-03-24 22:30 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: David Rowley <[email protected]>; Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On Mon, Mar 24, 2025 at 3:15 PM Tom Lane <[email protected]> wrote:
> FWIW, I share these doubts about whether these values are useful
> enough to include in the default EXPLAIN output. My main beef
> with them though is that they are basically numbers derived along
> the way to producing a cost estimate, and I don't think we break
> out such intermediate results for other node types.
>
The main argument I had initially when proposing this, is that Memoize is
different from other plan nodes, in that it makes the child node costs
"cheaper". Clearly seeing the expected cache hit/ratio (that drives that
costing modification) helps interpret why the planner came up with a given
plan.
Put differently, the reason this should be in the default EXPLAIN output
(or at least the VERBOSE output), is because Memoize's impact on costing is
counterintuitive (in my experience), and breaks the user's understanding of
planner costs you can usually derive by looking at the per-node cost
details, which typically flows up (i.e. gets larger as you step higher up
in the tree).
>
> It's looking like Robert's "pg_overexplain" will hit the tree soon,
> so maybe there could be a case for teaching that to emit additional
> costing details such as these?
>
I don't think that addresses the end-user usability sufficiently - from
what I gathered "pg_overexplain" was meant for a hacker audience, not
DBAs/etc interpreting Postgres plan choices.
Thanks,
Lukas
--
Lukas Fittl
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
@ 2025-03-24 22:45 ` Tom Lane <[email protected]>
2025-03-25 09:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: Tom Lane @ 2025-03-24 22:45 UTC (permalink / raw)
To: Lukas Fittl <[email protected]>; +Cc: David Rowley <[email protected]>; Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
Lukas Fittl <[email protected]> writes:
> The main argument I had initially when proposing this, is that Memoize is
> different from other plan nodes, in that it makes the child node costs
> "cheaper". Clearly seeing the expected cache hit/ratio (that drives that
> costing modification) helps interpret why the planner came up with a given
> plan.
Memoize is hardly unique in that respect. Merge Join sometimes
expects that it won't have to read the inner input to completion,
and reduces its cost estimate accordingly, and that confuses people.
LIMIT also reduces the cost estimate of its input, though perhaps
that doesn't surprise people.
As I said, I'm not necessarily averse to showing these numbers
somehow. But I don't think they belong in the default output,
and I'm not even convinced that VERBOSE is the right place.
pg_overexplain seems like it could be an ideal home for this
sort of detail.
regards, tom lane
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
@ 2025-03-25 09:32 ` Andrei Lepikhov <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-25 09:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Lukas Fittl <[email protected]>; +Cc: David Rowley <[email protected]>; Ilia Evdokimov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On 3/24/25 23:45, Tom Lane wrote:
> Lukas Fittl <[email protected]> writes:
>> The main argument I had initially when proposing this, is that Memoize is
>> different from other plan nodes, in that it makes the child node costs
>> "cheaper". Clearly seeing the expected cache hit/ratio (that drives that
>> costing modification) helps interpret why the planner came up with a given
>> plan.
> As I said, I'm not necessarily averse to showing these numbers
> somehow. But I don't think they belong in the default output,
> and I'm not even convinced that VERBOSE is the right place.
> pg_overexplain seems like it could be an ideal home for this
> sort of detail.
I prefer not to see these numbers in the default EXPLAIN output, not
only because they can fluctuate but mainly because I like to view the
basic query structure without additional details.
As I mentioned earlier, most of the information we typically need to
explore query plans stays in path nodes and does not transfer to the
Plan node. I believe this should stay as is, as we deal with numerous
cases and a vast amount of data.
It would be beneficial to expose extra data in an external extension. By
implementing a `create_plan` hook and an extensible list node in both
Path and Plan structures, we could provide a machinery for writing an
extension that can expose any planning-stage information in EXPLAIN on
demand.
This could encourage developers to create a "pg_extended_explain"
extension, which would address most user needs without requiring
additional changes to the core system.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
@ 2025-03-26 19:37 ` Robert Haas <[email protected]>
2025-03-27 10:12 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Robert Haas @ 2025-03-26 19:37 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Lukas Fittl <[email protected]>; David Rowley <[email protected]>; Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On Mon, Mar 24, 2025 at 6:46 PM Tom Lane <[email protected]> wrote:
> As I said, I'm not necessarily averse to showing these numbers
> somehow. But I don't think they belong in the default output,
> and I'm not even convinced that VERBOSE is the right place.
> pg_overexplain seems like it could be an ideal home for this
> sort of detail.
I think we're going to be sad if we start shunting things that
non-developers need into pg_overexplain. On the one hand, PostgreSQL
consultants will be annoyed because they'll have to get a contrib
module installed in order to be able to do their jobs, and I don't
think that should be a requirement. On the other hand, PostgreSQL
developers will also be annoyed because once the consultants start
using it they'll complain when we change things, and I think we want
to have the freedom to change things in pg_overexplain. For that
reason, I think that if we choose to display anything here, it should
either be displayed all the time or gated by some in-core option such
as VERBOSE.
I do acknowledge the argument that we don't show details of how costs
are derived in other cases. While I think that point has some
validity, the flip side is that I spend a fairly significant amount of
time attempting to reverse-engineer what the planner did from the
EXPLAIN output, and I find that pretty unenjoyable. The recent change
to show two decimal places on row-count estimation is one that comes
up a heck of a lot, and several people have thanked me for getting
that patch committed because that problem affected them, too. But it's
surely not the only example of a case where it's hard to determine
what happened in the planner from what shows up in EXPLAIN output, and
I think that trying to find ways to improve on that situation is
worthwhile.
I also don't think that we should be too concerned about bloating the
EXPLAIN output in the context of a patch that only affects Memoize.
Memoize nodes are not incredibly common in the query plans that I see,
so even if we added another line or three to the output for each one,
I don't think that would create major problems. On the other hand,
maybe there's an argument that what this patch touches is only the tip
of the iceberg, and that we're eventually going to want the same kinds
of things for Nested Loop and Hash Joins and Merge Joins, and maybe
even more detail that can be displayed in say 3 lines. In that case,
there's a double concern. On the one hand, that really would make the
output a whole lot more verbose, and on the other hand, it might
generate a fair amount of work to maintain it across future planner
changes. I can see deciding to reject changes of that sort on the
grounds that we're not prepared to maintain it, or deciding to gate it
behind a new option on the grounds that it is so verbose that even
people who say EXPLAIN VERBOSE are going to be sad if they get all
that crap by default. I'm not saying that we SHOULD make those
decisions -- I think exposing more detail here could be pretty useful
to people trying to solve query plan problems, including me, so I hope
we don't just kick that idea straight to the curb without due thought
-- but I would understand them.
The part I'm least sure about with respect to the proposed patch is
the actual stuff being displayed. I don't have the experience to know
whether it's useful for tracking down issues. If it's not, then I
agree we shouldn't display it. If it is, then I'm tentatively in favor
of showing it in standard EXPLAIN, possibly only with VERBOSE, with
the caveats from the previous paragraph: if more-common node types are
also going to have a bunch of stuff like this, then we need to think
more carefully. If Memoize is exceptional in needing additional
information displayed, then I think it's fine.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
@ 2025-03-27 10:12 ` Ilia Evdokimov <[email protected]>
2025-03-27 12:46 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
2025-03-27 13:46 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-27 10:12 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Lukas Fittl <[email protected]>; David Rowley <[email protected]>; Andrei Lepikhov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On 26.03.2025 22:37, Robert Haas wrote:
> I also don't think that we should be too concerned about bloating the
> EXPLAIN output in the context of a patch that only affects Memoize.
> Memoize nodes are not incredibly common in the query plans that I see,
> so even if we added another line or three to the output for each one,
> I don't think that would create major problems. On the other hand,
> maybe there's an argument that what this patch touches is only the tip
> of the iceberg, and that we're eventually going to want the same kinds
> of things for Nested Loop and Hash Joins and Merge Joins, and maybe
> even more detail that can be displayed in say 3 lines. In that case,
> there's a double concern. On the one hand, that really would make the
> output a whole lot more verbose, and on the other hand, it might
> generate a fair amount of work to maintain it across future planner
> changes. I can see deciding to reject changes of that sort on the
> grounds that we're not prepared to maintain it, or deciding to gate it
> behind a new option on the grounds that it is so verbose that even
> people who say EXPLAIN VERBOSE are going to be sad if they get all
> that crap by default. I'm not saying that we SHOULD make those
> decisions -- I think exposing more detail here could be pretty useful
> to people trying to solve query plan problems, including me, so I hope
> we don't just kick that idea straight to the curb without due thought
> -- but I would understand them.
>
> The part I'm least sure about with respect to the proposed patch is
> the actual stuff being displayed. I don't have the experience to know
> whether it's useful for tracking down issues. If it's not, then I
> agree we shouldn't display it. If it is, then I'm tentatively in favor
> of showing it in standard EXPLAIN, possibly only with VERBOSE, with
> the caveats from the previous paragraph: if more-common node types are
> also going to have a bunch of stuff like this, then we need to think
> more carefully. If Memoize is exceptional in needing additional
> information displayed, then I think it's fine.
>
> --
> Robert Haas
> EDB:http://www.enterprisedb.com
I understand the concerns raised about the risk of opening the door to
more diagnostic detail across various plan nodes. However, in Hash Join,
Merge Join, and Nested Loop, EXPLAIN typically reveals at least some of
the planner’s expectations. For example, Hash Join shows the number of
batches and originally expected buckets, giving insight into whether the
hash table fit in memory. Merge Join shows unexpected Sort nodes when
presorted inputs were assumed. Nested Loop reflects planner assumptions
via loops and row estimates. In other words, these nodes expose at least
some information about what the planner thought would happen.
Memoize is unique in that it shows runtime statistics (hits, misses,
evictions), but reveals nothing about the planner’s expectations. We
don’t see how many distinct keys were estimated or how many entries the
planner thought would fit in memory. This makes it very difficult to
understand whether Memoize was a good choice or not, or how to fix it
when it performs poorly.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
2025-03-27 10:12 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
@ 2025-03-27 12:46 ` Robert Haas <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Robert Haas @ 2025-03-27 12:46 UTC (permalink / raw)
To: Ilia Evdokimov <[email protected]>; +Cc: Tom Lane <[email protected]>; Lukas Fittl <[email protected]>; David Rowley <[email protected]>; Andrei Lepikhov <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On Thu, Mar 27, 2025 at 6:12 AM Ilia Evdokimov
<[email protected]> wrote:
> I understand the concerns raised about the risk of opening the door to more diagnostic detail across various plan nodes. However, in Hash Join, Merge Join, and Nested Loop, EXPLAIN typically reveals at least some of the planner’s expectations. For example, Hash Join shows the number of batches and originally expected buckets, giving insight into whether the hash table fit in memory. Merge Join shows unexpected Sort nodes when presorted inputs were assumed. Nested Loop reflects planner assumptions via loops and row estimates. In other words, these nodes expose at least some information about what the planner thought would happen.
>
> Memoize is unique in that it shows runtime statistics (hits, misses, evictions), but reveals nothing about the planner’s expectations. We don’t see how many distinct keys were estimated or how many entries the planner thought would fit in memory. This makes it very difficult to understand whether Memoize was a good choice or not, or how to fix it when it performs poorly.
Right. Without taking a strong position on this particular patch, I
think it's entirely reasonable, as a concept, to think of making
Memoize work similarly to what other nodes already do.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
2025-03-27 10:12 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
@ 2025-03-27 13:46 ` Andrei Lepikhov <[email protected]>
2025-03-28 12:20 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-27 13:46 UTC (permalink / raw)
To: Ilia Evdokimov <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Lukas Fittl <[email protected]>; David Rowley <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On 3/27/25 11:12, Ilia Evdokimov wrote:
> Memoize is unique in that it shows runtime statistics (hits, misses,
> evictions), but reveals nothing about the planner’s expectations. We
> don’t see how many distinct keys were estimated or how many entries the
> planner thought would fit in memory.
It is acceptable for me to have Memoize estimations covered by the COSTS
option.
This data may help both developers and the users. For us, it provides
information on how good group estimations are and lets us identify gaps
in the prediction code. A user may find insights about ways to optimise
the query, detecting stale statistics or defining extended statistics.
This makes it very difficult to
> understand whether Memoize was a good choice or not, or how to fix it
> when it performs poorly.
I think user already has enough data to determine whether Memoize was
the right choice - hits/misses/evictions show that.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
2025-03-27 10:12 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-27 13:46 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
@ 2025-03-28 12:20 ` Ilia Evdokimov <[email protected]>
2025-04-01 07:52 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Ilia Evdokimov @ 2025-03-28 12:20 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Lukas Fittl <[email protected]>; David Rowley <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
Then we need to decide clearly what exactly to display in EXPLAIN for
the Memoize node: absolute values (estimated distinct keys and estimated
cache capacity) or ratios (hit_ratio and evict_ratio). Ratios have the
advantage of quickly reflecting the overall effectiveness of Memoize.
However, absolute values have a significant advantage as they explicitly
reveal the reason of Memoize's poor performance, making problem
diagnosis simpler.
With absolute values, users can directly understand the underlying
reason for poor performance. For example: insufficient memory (capacity
< distinct keys), inaccurate planner statistics (distinct keys
significantly different from actual values), poorly ordered keys
(capacity ~ distinct keys, but frequent evictions as seen in the
Evictions parameter), or Memoize simply not being beneficial (capacity ~
distinct keys ~ calls). Ratios, by contrast, only reflect the final
outcome without clearly indicating the cause or the specific steps
needed to resolve the issue.
Thus, absolute values do more than just inform users that a problem
exists; they provide actionable details that enable users to directly
address the problem (increase work_mem, refresh statistics, create
extended statistics, or disable Memoize entirely). Additionally, no
other plan nodes in PostgreSQL currently use a similar ratio-based
approach - everywhere else absolute values are consistently shown (e.g.,
number of rows, buckets, batches, memory used, etc.). Using absolute
values in Memoize maintains consistency with existing practice.
I've updated the patch to v5, since the new parameter est_unique_keys in
make_memoize() is now placed near est_entries, which is more logical and
readable than putting it at the end.
Any thoughts?
--
Best Regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v5-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch (5.7K, ../../[email protected]/2-v5-0001-Show-ndistinct-and-est_entries-in-EXPLAIN-for-Memoize.patch)
download | inline diff:
From 79af33e499730a4bd0553832b23bf68c90817c6e Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <[email protected]>
Date: Fri, 28 Mar 2025 15:17:09 +0300
Subject: [PATCH v5] Show ndistinct and est_entries in EXPLAIN for Memoize
---
src/backend/commands/explain.c | 16 ++++++++++++++++
src/backend/optimizer/path/costsize.c | 3 +++
src/backend/optimizer/plan/createplan.c | 10 +++++++---
src/backend/optimizer/util/pathnode.c | 3 +++
src/include/nodes/pathnodes.h | 2 ++
src/include/nodes/plannodes.h | 6 ++++++
6 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 391b34a2af2..2bacf7f85cb 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3628,6 +3628,22 @@ show_memoize_info(MemoizeState *mstate, List *ancestors, ExplainState *es)
ExplainPropertyText("Cache Key", keystr.data, es);
ExplainPropertyText("Cache Mode", mstate->binary_mode ? "binary" : "logical", es);
+ if (es->costs)
+ {
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ ExplainIndentText(es);
+ appendStringInfo(es->str, "Estimated Capacity: %u Estimated Distinct Lookup Keys: %0.0f\n",
+ ((Memoize *) plan)->est_entries,
+ ((Memoize *) plan)->est_unique_keys);
+ }
+ else
+ {
+ ExplainPropertyUInteger("Estimated Capacity", "", ((Memoize *) plan)->est_entries, es);
+ ExplainPropertyFloat("Estimated Distinct Lookup Keys", "", ((Memoize *) plan)->est_unique_keys, 0, es);
+ }
+ }
+
pfree(keystr.data);
if (!es->analyze)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index f6f77b8fe19..e3abf9ccc26 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2604,6 +2604,9 @@ cost_memoize_rescan(PlannerInfo *root, MemoizePath *mpath,
mpath->est_entries = Min(Min(ndistinct, est_cache_entries),
PG_UINT32_MAX);
+ /* Remember ndistinct for a potential EXPLAIN later */
+ mpath->est_unique_keys = ndistinct;
+
/*
* When the number of distinct parameter values is above the amount we can
* store in the cache, then we'll have to evict some entries from the
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 75e2b0b9036..38882501484 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -284,7 +284,8 @@ static Material *make_material(Plan *lefttree);
static Memoize *make_memoize(Plan *lefttree, Oid *hashoperators,
Oid *collations, List *param_exprs,
bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids);
+ uint32 est_entries, double est_unique_keys,
+ Bitmapset *keyparamids);
static WindowAgg *make_windowagg(List *tlist, WindowClause *wc,
int partNumCols, AttrNumber *partColIdx, Oid *partOperators, Oid *partCollations,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
@@ -1703,7 +1704,8 @@ create_memoize_plan(PlannerInfo *root, MemoizePath *best_path, int flags)
plan = make_memoize(subplan, operators, collations, param_exprs,
best_path->singlerow, best_path->binary_mode,
- best_path->est_entries, keyparamids);
+ best_path->est_entries, best_path->est_unique_keys,
+ keyparamids);
copy_generic_path_info(&plan->plan, (Path *) best_path);
@@ -6636,7 +6638,8 @@ materialize_finished_plan(Plan *subplan)
static Memoize *
make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
List *param_exprs, bool singlerow, bool binary_mode,
- uint32 est_entries, Bitmapset *keyparamids)
+ uint32 est_entries, double est_unique_keys,
+ Bitmapset *keyparamids)
{
Memoize *node = makeNode(Memoize);
Plan *plan = &node->plan;
@@ -6654,6 +6657,7 @@ make_memoize(Plan *lefttree, Oid *hashoperators, Oid *collations,
node->binary_mode = binary_mode;
node->est_entries = est_entries;
node->keyparamids = keyparamids;
+ node->est_unique_keys = est_unique_keys;
return node;
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..1fbcda99067 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1701,6 +1701,9 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
Assert(enable_memoize);
pathnode->path.disabled_nodes = subpath->disabled_nodes;
+ /* Estimated number of distinct memoization keys, computed using estimate_num_groups() */
+ pathnode->est_unique_keys = 0;
+
/*
* Add a small additional charge for caching the first entry. All the
* harder calculations for rescans are performed in cost_memoize_rescan().
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c24a1fc8514..0946ccea994 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2040,6 +2040,8 @@ typedef struct MemoizePath
uint32 est_entries; /* The maximum number of entries that the
* planner expects will fit in the cache, or 0
* if unknown */
+ double est_unique_keys; /* Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN */
} MemoizePath;
/*
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 658d76225e4..3d9d3a1159d 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1063,6 +1063,12 @@ typedef struct Memoize
/* paramids from param_exprs */
Bitmapset *keyparamids;
+
+ /*
+ * Estimated number of distinct memoization keys,
+ * used for cache size evaluation. Kept for EXPLAIN
+ */
+ double est_unique_keys;
} Memoize;
/* ----------------
--
2.34.1
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-24 22:30 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
2025-03-26 19:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Robert Haas <[email protected]>
2025-03-27 10:12 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-27 13:46 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-28 12:20 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
@ 2025-04-01 07:52 ` Ilia Evdokimov <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Ilia Evdokimov @ 2025-04-01 07:52 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; David Rowley <[email protected]>; +Cc: Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>; Andrei Lepikhov <[email protected]>
On 28.03.2025 15:20, Ilia Evdokimov wrote:
> Then we need to decide clearly what exactly to display in EXPLAIN for
> the Memoize node: absolute values (estimated distinct keys and
> estimated cache capacity) or ratios (hit_ratio and evict_ratio).
> Ratios have the advantage of quickly reflecting the overall
> effectiveness of Memoize. However, absolute values have a significant
> advantage as they explicitly reveal the reason of Memoize's poor
> performance, making problem diagnosis simpler.
>
> With absolute values, users can directly understand the underlying
> reason for poor performance. For example: insufficient memory
> (capacity < distinct keys), inaccurate planner statistics (distinct
> keys significantly different from actual values), poorly ordered keys
> (capacity ~ distinct keys, but frequent evictions as seen in the
> Evictions parameter), or Memoize simply not being beneficial (capacity
> ~ distinct keys ~ calls). Ratios, by contrast, only reflect the final
> outcome without clearly indicating the cause or the specific steps
> needed to resolve the issue.
>
> Thus, absolute values do more than just inform users that a problem
> exists; they provide actionable details that enable users to directly
> address the problem (increase work_mem, refresh statistics, create
> extended statistics, or disable Memoize entirely). Additionally, no
> other plan nodes in PostgreSQL currently use a similar ratio-based
> approach - everywhere else absolute values are consistently shown
> (e.g., number of rows, buckets, batches, memory used, etc.). Using
> absolute values in Memoize maintains consistency with existing practice.
>
> I've updated the patch to v5, since the new parameter est_unique_keys
> in make_memoize() is now placed near est_entries, which is more
> logical and readable than putting it at the end.
>
> Any thoughts?
>
> --
> Best Regards,
> Ilia Evdokimov,
> Tantor Labs LLC.
With the feature freeze coming up soon, I’d like to ask: do we plan to
include this patch in v18?
Please let me know if there’s anything I can do to help move it forward.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 22:15 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Tom Lane <[email protected]>
@ 2025-03-25 03:32 ` David Rowley <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: David Rowley @ 2025-03-25 03:32 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrei Lepikhov <[email protected]>; Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On Tue, 25 Mar 2025 at 11:15, Tom Lane <[email protected]> wrote:
> FWIW, I share these doubts about whether these values are useful
> enough to include in the default EXPLAIN output. My main beef
> with them though is that they are basically numbers derived along
> the way to producing a cost estimate, and I don't think we break
> out such intermediate results for other node types.
The only likeness I can think of is the "(originally N)" for the
buckets and batches in EXPLAIN ANALYZE for Hash Joins. I see that that
only shows when one of those isn't the same as the planner's
expectations
Maybe there's room to show additional details for Memoize in some
similar way only during EXPLAIN ANALYZE. EXPLAIN ANALYZE for Memoize
currently shows things like the number of hits, misses and evictions.
We could calculate what the planner expected those values to be and
show those. For this case, they're almost certain not to match what
the planner expected, but maybe that's ok.
David
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2023-07-06 07:56 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-20 12:32 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-21 09:02 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-23 21:16 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
2025-03-24 21:23 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment David Rowley <[email protected]>
@ 2025-03-25 06:40 ` Andrei Lepikhov <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Andrei Lepikhov @ 2025-03-25 06:40 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Lukas Fittl <[email protected]>; Daniel Gustafsson <[email protected]>; PostgreSQL Hackers <[email protected]>; Nikolay Samokhvalov <[email protected]>
On 24/3/2025 23:05, David Rowley wrote:
> On Tue, 25 Mar 2025 at 10:23, Andrei Lepikhov <[email protected]> wrote:
>>
>> On 3/23/25 22:16, David Rowley wrote:
>>> Once again, I'm not necessarily objecting to hit and evict ratios
>>> being shown, I just want to know they're actually useful enough to
>>> show and don't just bloat the EXPLAIN output needlessly. So far your
>>> arguments aren't convincing me that they are.
>
>> I'm -1 for this redundancy.
>
> I'm not following what the -1 is for. Is it for showing both hit and
> evict ratios? And your vote is only for adding hit ratio?
Oh, pardon me. I wanted to say that IMO, the couple (capacity, distinct
lookup keys) makes redundant (hit ratio, eviction ratio) and vice versa.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
@ 2025-03-07 00:05 Jelte Fennema-Nio <[email protected]>
2025-03-07 08:08 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: Jelte Fennema-Nio @ 2025-03-07 00:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; pgsql-hackers
On Tue, 25 Feb 2025 at 02:11, Michael Paquier <[email protected]> wrote:
> Initial digestion has gone well.
One thing I've noticed is that \startpipeline throws warnings when
copy pasting multiple lines. It seems to still execute everything as
expected though. As an example you can copy paste this tiny script:
\startpipeline
select pg_sleep(5) \bind \g
\endpipeline
And then it will show these "extra argument ... ignored" warnings
\startpipeline: extra argument "select" ignored
\startpipeline: extra argument "pg_sleep(5)" ignored
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
@ 2025-03-07 08:08 ` Anthonin Bonnefoy <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Anthonin Bonnefoy @ 2025-03-07 08:08 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Michael Paquier <[email protected]>; pgsql-hackers
On Thu, Mar 6, 2025 at 5:20 AM Michael Paquier <[email protected]> wrote:
> That was not a test case we had in mind originally here, but if it is
> possible to keep the implementation simple while supporting your
> demand, well, let's do it. If it's not that straight-forward, let's
> use the new meta-command, forbidding \g and \gx based on your
> arguments from upthread.
I think the new meta-command is a separate issue from allowing ';' to
push in a pipeline. Any time there's a change or an additional format
option added to \g, it will need to be forbidden for pipelining. The
\sendpipeline meta-command will help keep those exceptions low since
the whole \g will be forbidden.
Another possible option would be to allow both \g and \gx, but send a
warning like "printing options within a pipeline will be ignored" if
those options are used, similar to "SET LOCAL" warning when done
outside of a transaction block. That would have the benefit of making
existing scripts using \g and \gx compatible.
For using ';' to push commands in a pipeline, I think it should be
fairly straightforward. I can try to work on that next week (I'm
currently chasing a weird memory context bug that I need to finish
first).
On Fri, Mar 7, 2025 at 1:05 AM Jelte Fennema-Nio <[email protected]> wrote:
> One thing I've noticed is that \startpipeline throws warnings when
> copy pasting multiple lines. It seems to still execute everything as
> expected though. As an example you can copy paste this tiny script:
>
> \startpipeline
> select pg_sleep(5) \bind \g
> \endpipeline
>
> And then it will show these "extra argument ... ignored" warnings
>
> \startpipeline: extra argument "select" ignored
> \startpipeline: extra argument "pg_sleep(5)" ignored
It looks like an issue with libreadline. At least, I've been able to
reproduce the warnings and 'readline(prompt);' returns everything as a
single line, with the \n inside the string. This explains why what is
after \startpipeline is processed as arguments. This can also be done
with:
select 1 \bind \g
select 2 \bind \g
And somehow, I couldn't reproduce the issue anymore once I've compiled
and installed libreadline with debug symbols.
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
@ 2025-03-07 22:31 ` Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Daniel Verite @ 2025-03-07 22:31 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Michael Paquier <[email protected]>; Anthonin Bonnefoy <[email protected]>; pgsql-hackers
Jelte Fennema-Nio wrote:
> As an example you can copy paste this tiny script:
>
> \startpipeline
> select pg_sleep(5) \bind \g
> \endpipeline
>
> And then it will show these "extra argument ... ignored" warnings
>
> \startpipeline: extra argument "select" ignored
> \startpipeline: extra argument "pg_sleep(5)" ignored
It happens with other metacommands as well, and appears
to depend on a readline option that is "on" by default since
readline-8.1 [1]
enable-bracketed-paste
When set to ‘On’, Readline configures the terminal to insert each
paste into the editing buffer as a single string of characters,
instead of treating each character as if it had been read from the
keyboard. This is called putting the terminal into bracketed paste
mode; it prevents Readline from executing any editing commands
bound to key sequences appearing in the pasted text. The default
is ‘On’.
This behavior of the metacommand complaining about arguments
on the next line also happens if using \e and typing this sequence
of commands in the editor. In that case readline is not involved.
There might be something to improve here, because
a metacommand cannot take its argument from the next line,
and yet that's what the error messages somewhat imply.
But that issue is not related to the new pipeline metacommands.
[1]
https://tiswww.case.edu/php/chet/readline/readline.html#index-enable_002dbracketed_002dpaste
Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
@ 2025-03-17 09:50 ` Anthonin Bonnefoy <[email protected]>
2025-03-17 10:19 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 09:36 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
0 siblings, 3 replies; 50+ messages in thread
From: Anthonin Bonnefoy @ 2025-03-17 09:50 UTC (permalink / raw)
To: Daniel Verite <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers
Here is a new patch set:
0001: This introduces the \sendpipeline meta-command and forbid \g in
a pipeline. This is to fix the formatting options of \g that are not
supported in a pipeline.
0002: Allows ';' to send a query using extended protocol when within a
pipeline by using PQsendQueryParams with 0 parameters. It is not
possible to send parameters with extended protocol this way and
everything will be propagated through the query string, similar to a
simple query.
Attachments:
[application/octet-stream] v02-0001-psql-Create-new-sendpipeline-meta-command.patch (38.8K, ../../CAO6_Xqq4JEHRaGyC=r+dpi0Ejbp2P3aUo-KfR2NuipCvE-o_ZA@mail.gmail.com/2-v02-0001-psql-Create-new-sendpipeline-meta-command.patch)
download | inline diff:
From 097084856575dbcd53baad01a1b5420b4201036d Mon Sep 17 00:00:00 2001
From: Anthonin Bonnefoy <[email protected]>
Date: Tue, 4 Mar 2025 09:47:45 +0100
Subject: psql: Create new \sendpipeline meta-command
In the initial pipelining support for psql, \g was reused as a way to
push extended query into an ongoing pipeline. However, all related
meta-command options are not applicable in pipeline mode. Pipeline
output only happens during \getresults or \endpipeline and any change to
output options would have been reset.
To avoid possible confusion, a new \sendpipeline meta-command is
introduced, replacing \g when within a pipeline.
---
doc/src/sgml/ref/psql-ref.sgml | 11 +-
src/bin/psql/command.c | 45 +++-
src/bin/psql/help.c | 1 +
src/bin/psql/tab-complete.in.c | 2 +-
src/test/regress/expected/psql.out | 1 +
src/test/regress/expected/psql_pipeline.out | 245 +++++++++++---------
src/test/regress/sql/psql.sql | 1 +
src/test/regress/sql/psql_pipeline.sql | 230 +++++++++---------
8 files changed, 310 insertions(+), 226 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index cedccc14129..cddf6e07531 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3677,6 +3677,7 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
<varlistentry id="app-psql-meta-command-pipeline">
<term><literal>\startpipeline</literal></term>
+ <term><literal>\sendpipeline</literal></term>
<term><literal>\syncpipeline</literal></term>
<term><literal>\endpipeline</literal></term>
<term><literal>\flushrequest</literal></term>
@@ -3701,10 +3702,10 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
queries need to be sent using the meta-commands
<literal>\bind</literal>, <literal>\bind_named</literal>,
<literal>\close</literal> or <literal>\parse</literal>. While a
- pipeline is ongoing, <literal>\g</literal> will append the current
- query buffer to the pipeline. Other meta-commands like
- <literal>\gx</literal> or <literal>\gdesc</literal> are not allowed
- in pipeline mode.
+ pipeline is ongoing, <literal>\sendpipeline</literal> will append the
+ current query buffer to the pipeline. Other meta-commands like
+ <literal>\g</literal>, <literal>\gx</literal> or <literal>\gdesc</literal>
+ are not allowed in pipeline mode.
</para>
<para>
@@ -3738,7 +3739,7 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
Example:
<programlisting>
\startpipeline
-SELECT 1 \bind \g
+SELECT 1 \bind \sendpipeline
\flushrequest
\getresults
\endpipeline
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index fb0b27568c5..7670c20b29e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -137,6 +137,7 @@ static backslashResult exec_command_setenv(PsqlScanState scan_state, bool active
static backslashResult exec_command_sf_sv(PsqlScanState scan_state, bool active_branch,
const char *cmd, bool is_func);
static backslashResult exec_command_startpipeline(PsqlScanState scan_state, bool active_branch);
+static backslashResult exec_command_sendpipeline(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_syncpipeline(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_t(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_T(PsqlScanState scan_state, bool active_branch);
@@ -427,6 +428,8 @@ exec_command(const char *cmd,
status = exec_command_sf_sv(scan_state, active_branch, cmd, false);
else if (strcmp(cmd, "startpipeline") == 0)
status = exec_command_startpipeline(scan_state, active_branch);
+ else if (strcmp(cmd, "sendpipeline") == 0)
+ status = exec_command_sendpipeline(scan_state, active_branch);
else if (strcmp(cmd, "syncpipeline") == 0)
status = exec_command_syncpipeline(scan_state, active_branch);
else if (strcmp(cmd, "t") == 0)
@@ -1734,10 +1737,9 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
if (status == PSQL_CMD_SKIP_LINE && active_branch)
{
- if (strcmp(cmd, "gx") == 0 &&
- PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
{
- pg_log_error("\\gx not allowed in pipeline mode");
+ pg_log_error("\\%s not allowed in pipeline mode", cmd);
clean_extended_state();
free(fname);
return PSQL_CMD_ERROR;
@@ -2979,6 +2981,43 @@ exec_command_startpipeline(PsqlScanState scan_state, bool active_branch)
return status;
}
+/*
+ * \sendpipeline -- send an extended query to an ongoing pipeline
+ */
+static backslashResult
+exec_command_sendpipeline(PsqlScanState scan_state, bool active_branch)
+{
+ backslashResult status = PSQL_CMD_SKIP_LINE;
+
+ if (active_branch)
+ {
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ {
+ if (pset.send_mode == PSQL_SEND_EXTENDED_QUERY_PREPARED ||
+ pset.send_mode == PSQL_SEND_EXTENDED_QUERY_PARAMS)
+ {
+ status = PSQL_CMD_SEND;
+ }
+ else
+ {
+ pg_log_error("\\sendpipeline must be used after \\bind or \\bind_named");
+ clean_extended_state();
+ return PSQL_CMD_ERROR;
+ }
+ }
+ else
+ {
+ pg_log_error("\\sendpipeline not allowed outside of pipeline mode");
+ clean_extended_state();
+ return PSQL_CMD_ERROR;
+ }
+ }
+ else
+ ignore_slash_options(scan_state);
+
+ return status;
+}
+
/*
* \syncpipeline -- send a sync message to an active pipeline
*/
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 714b8619233..edfc794b76f 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -182,6 +182,7 @@ slashUsage(unsigned short int pager)
HELP0(" \\parse STMT_NAME create a prepared statement\n");
HELP0(" \\q quit psql\n");
HELP0(" \\startpipeline enter pipeline mode\n");
+ HELP0(" \\sendpipeline send an extended query to an ongoing pipeline\n");
HELP0(" \\syncpipeline add a synchronisation point to an ongoing pipeline\n");
HELP0(" \\watch [[i=]SEC] [c=N] [m=MIN]\n"
" execute query every SEC seconds, up to N times,\n"
diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c
index 8432be641ac..b2de63a5b74 100644
--- a/src/bin/psql/tab-complete.in.c
+++ b/src/bin/psql/tab-complete.in.c
@@ -1895,7 +1895,7 @@ psql_completion(const char *text, int start, int end)
"\\parse", "\\password", "\\print", "\\prompt", "\\pset",
"\\qecho", "\\quit",
"\\reset",
- "\\s", "\\set", "\\setenv", "\\sf", "\\startpipeline", "\\sv", "\\syncpipeline",
+ "\\s", "\\set", "\\setenv", "\\sf", "\\startpipeline", "\\sendpipeline", "\\sv", "\\syncpipeline",
"\\t", "\\T", "\\timing",
"\\unset",
"\\x",
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 6543e90de75..ee6f8671bea 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -4711,6 +4711,7 @@ invalid command \lo
\sf whole_line
\sv whole_line
\startpipeline
+ \sendpipeline
\syncpipeline
\t arg1
\T arg1
diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out
index 3df2415a840..53f2edfd986 100644
--- a/src/test/regress/expected/psql_pipeline.out
+++ b/src/test/regress/expected/psql_pipeline.out
@@ -4,7 +4,7 @@
CREATE TABLE psql_pipeline(a INTEGER PRIMARY KEY, s TEXT);
-- Single query
\startpipeline
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
?column?
----------
@@ -13,9 +13,9 @@ SELECT $1 \bind 'val1' \g
-- Multiple queries
\startpipeline
-SELECT $1 \bind 'val1' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\endpipeline
?column?
----------
@@ -35,10 +35,10 @@ SELECT $1, $2 \bind 'val2' 'val3' \g
-- Test \flush
\startpipeline
\flush
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\flush
-SELECT $1, $2 \bind 'val2' 'val3' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\endpipeline
?column?
----------
@@ -63,12 +63,12 @@ SELECT $1, $2 \bind 'val2' 'val3' \g
0
\echo :PIPELINE_RESULT_COUNT
0
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\syncpipeline
\syncpipeline
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\syncpipeline
-SELECT $1, $2 \bind 'val4' 'val5' \g
+SELECT $1, $2 \bind 'val4' 'val5' \sendpipeline
\echo :PIPELINE_COMMAND_COUNT
1
\echo :PIPELINE_SYNC_COUNT
@@ -94,7 +94,7 @@ SELECT $1, $2 \bind 'val4' 'val5' \g
-- \startpipeline should not have any effect if already in a pipeline.
\startpipeline
\startpipeline
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
?column?
----------
@@ -103,24 +103,24 @@ SELECT $1 \bind 'val1' \g
-- Convert an implicit transaction block to an explicit transaction block.
\startpipeline
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 2 \g
-ROLLBACK \bind \g
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 2 \sendpipeline
+ROLLBACK \bind \sendpipeline
\endpipeline
-- Multiple explicit transactions
\startpipeline
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-ROLLBACK \bind \g
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-COMMIT \bind \g
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+ROLLBACK \bind \sendpipeline
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+COMMIT \bind \sendpipeline
\endpipeline
-- COPY FROM STDIN
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\endpipeline
?column?
----------
@@ -129,8 +129,8 @@ COPY psql_pipeline FROM STDIN \bind \g
-- COPY FROM STDIN with \flushrequest + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\flushrequest
\getresults
?column?
@@ -142,8 +142,8 @@ message type 0x5a arrived from server while idle
\endpipeline
-- COPY FROM STDIN with \syncpipeline + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\syncpipeline
\getresults
?column?
@@ -154,8 +154,8 @@ COPY psql_pipeline FROM STDIN \bind \g
\endpipeline
-- COPY TO STDOUT
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\endpipeline
?column?
----------
@@ -168,8 +168,8 @@ copy psql_pipeline TO STDOUT \bind \g
4 test4
-- COPY TO STDOUT with \flushrequest + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\flushrequest
\getresults
?column?
@@ -184,8 +184,8 @@ copy psql_pipeline TO STDOUT \bind \g
\endpipeline
-- COPY TO STDOUT with \syncpipeline + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\syncpipeline
\getresults
?column?
@@ -203,14 +203,14 @@ copy psql_pipeline TO STDOUT \bind \g
SELECT $1 \parse ''
SELECT $1, $2 \parse ''
SELECT $2 \parse pipeline_1
-\bind_named '' 1 2 \g
-\bind_named pipeline_1 2 \g
+\bind_named '' 1 2 \sendpipeline
+\bind_named pipeline_1 2 \sendpipeline
\endpipeline
ERROR: could not determine data type of parameter $1
-- \getresults displays all results preceding a \flushrequest.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
\getresults
?column?
@@ -226,8 +226,8 @@ SELECT $1 \bind 2 \g
\endpipeline
-- \getresults displays all results preceding a \syncpipeline.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\syncpipeline
\getresults
?column?
@@ -245,7 +245,7 @@ SELECT $1 \bind 2 \g
\startpipeline
\getresults
No pending results to get
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
No pending results to get
\flushrequest
@@ -259,9 +259,9 @@ No pending results to get
No pending results to get
-- \getresults only fetches results preceding a \flushrequest.
\startpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
?column?
----------
@@ -276,9 +276,9 @@ SELECT $1 \bind 2 \g
-- \getresults only fetches results preceding a \syncpipeline.
\startpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\syncpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
?column?
----------
@@ -294,7 +294,7 @@ SELECT $1 \bind 2 \g
-- Use pipeline with chunked results for both \getresults and \endpipeline.
\startpipeline
\set FETCH_COUNT 10
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
\getresults
?column?
@@ -302,7 +302,7 @@ SELECT $1 \bind 2 \g
2
(1 row)
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
?column?
----------
@@ -312,9 +312,9 @@ SELECT $1 \bind 2 \g
\unset FETCH_COUNT
-- \getresults with specific number of requested results.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
-SELECT $1 \bind 3 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
+SELECT $1 \bind 3 \sendpipeline
\echo :PIPELINE_SYNC_COUNT
0
\syncpipeline
@@ -330,7 +330,7 @@ SELECT $1 \bind 3 \g
\echo :PIPELINE_RESULT_COUNT
2
-SELECT $1 \bind 4 \g
+SELECT $1 \bind 4 \sendpipeline
\getresults 3
?column?
----------
@@ -354,7 +354,7 @@ SELECT $1 \bind 4 \g
\startpipeline
\syncpipeline
\syncpipeline
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\flushrequest
\getresults 2
\getresults 1
@@ -366,9 +366,9 @@ SELECT $1 \bind 1 \g
\endpipeline
-- \getresults 0 should get all the results.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
-SELECT $1 \bind 3 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
+SELECT $1 \bind 3 \sendpipeline
\syncpipeline
\getresults 0
?column?
@@ -398,7 +398,7 @@ cannot send pipeline when not in pipeline mode
\startpipeline
SELECT 1;
PQsendQuery not allowed in pipeline mode
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
?column?
----------
@@ -408,9 +408,9 @@ SELECT $1 \bind 'val1' \g
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\syncpipeline
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
?column?
@@ -421,23 +421,23 @@ ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
-- For an incorrect number of parameters, the pipeline is aborted and
-- the following queries will not be executed.
\startpipeline
-SELECT \bind 'val1' \g
-SELECT $1 \bind 'val1' \g
+SELECT \bind 'val1' \sendpipeline
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
ERROR: bind message supplies 1 parameters, but prepared statement "" requires 0
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-ROLLBACK \bind \g
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+ROLLBACK \bind \sendpipeline
\endpipeline
ERROR: duplicate key value violates unique constraint "psql_pipeline_pkey"
DETAIL: Key (a)=(1) already exists.
ROLLBACK;
-- \watch sends a simple query, something not allowed within a pipeline.
\startpipeline
-SELECT \bind \g
+SELECT \bind \sendpipeline
\watch 1
PQsendQuery not allowed in pipeline mode
@@ -450,7 +450,7 @@ PQsendQuery not allowed in pipeline mode
\startpipeline
SELECT $1 \bind 1 \gdesc
synchronous command execution functions are not allowed in pipeline mode
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
?column?
----------
@@ -465,19 +465,33 @@ SELECT $1 as k, $2 as l \parse 'second'
\gset not allowed in pipeline mode
\bind_named second 1 2 \gset pref02_ \echo :pref02_i :pref02_j
\gset not allowed in pipeline mode
-\bind_named '' 1 2 \g
+\bind_named '' 1 2 \sendpipeline
\endpipeline
i | j
---+---
1 | 2
(1 row)
--- \gx is not allowed, pipeline should still be usable.
+-- \g and \gx are not allowed, pipeline should still be usable.
\startpipeline
+SELECT $1 \bind 1 \g
+\g not allowed in pipeline mode
+SELECT $1 \bind 1 \g filename
+\g not allowed in pipeline mode
+SELECT $1 \bind 1 \g |cat
+\g not allowed in pipeline mode
+SELECT $1 \bind 1 \g (format=unaligned tuples_only=on)
+\g not allowed in pipeline mode
SELECT $1 \bind 1 \gx
\gx not allowed in pipeline mode
+SELECT $1 \bind 1 \gx filename
+\gx not allowed in pipeline mode
+SELECT $1 \bind 1 \gx |cat
+\gx not allowed in pipeline mode
+SELECT $1 \bind 1 \gx (format=unaligned tuples_only=on)
+\gx not allowed in pipeline mode
\reset
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
?column?
----------
@@ -487,54 +501,63 @@ SELECT $1 \bind 1 \g
-- \gx warning should be emitted in an aborted pipeline, with
-- pipeline still usable.
\startpipeline
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\flushrequest
\getresults
ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
SELECT $1 \bind 1 \gx
\gx not allowed in pipeline mode
\endpipeline
+-- \sendpipeline is not allowed outside of a pipeline
+\sendpipeline
+\sendpipeline not allowed outside of pipeline mode
+SELECT $1 \bind 1 \sendpipeline
+\sendpipeline not allowed outside of pipeline mode
+\reset
+-- \sendpipeline is not allowed if not preceded by \bind or \bind_named
+\startpipeline
+\sendpipeline
+\sendpipeline must be used after \bind or \bind_named
+SELECT 1 \sendpipeline
+\sendpipeline must be used after \bind or \bind_named
+\endpipeline
-- \gexec is not allowed, pipeline should still be usable.
\startpipeline
SELECT 'INSERT INTO psql_pipeline(a) SELECT generate_series(1, 10)' \parse 'insert_stmt'
\bind_named insert_stmt \gexec
\gexec not allowed in pipeline mode
-\bind_named insert_stmt \g
+\bind_named insert_stmt \sendpipeline
SELECT COUNT(*) FROM psql_pipeline \bind \g
+\g not allowed in pipeline mode
\endpipeline
?column?
------------------------------------------------------------
INSERT INTO psql_pipeline(a) SELECT generate_series(1, 10)
(1 row)
- count
--------
- 4
-(1 row)
-
-- After an error, pipeline is aborted and requires \syncpipeline to be
-- reusable.
\startpipeline
-SELECT $1 \bind \g
-SELECT $1 \bind 1 \g
+SELECT $1 \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
\flushrequest
\getresults
ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
-- Pipeline is aborted.
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
-- Sync allows pipeline to recover.
\syncpipeline
\getresults
Pipeline aborted, command did not run
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
\flushrequest
\getresults
@@ -551,10 +574,10 @@ SELECT $1 \parse a
\endpipeline
-- In an aborted pipeline, \getresults 1 aborts commands one at a time.
\startpipeline
-SELECT $1 \bind \g
-SELECT $1 \bind 1 \g
+SELECT $1 \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\syncpipeline
\getresults 1
ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
@@ -569,11 +592,11 @@ Pipeline aborted, command did not run
-- Test chunked results with an aborted pipeline.
\startpipeline
\set FETCH_COUNT 10
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\flushrequest
\getresults
ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\endpipeline
fetching results in chunked mode failed
Pipeline aborted, command did not run
@@ -595,7 +618,7 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
-SELECT 1 \bind \g
+SELECT 1 \bind \sendpipeline
SELECT 1;
PQsendQuery not allowed in pipeline mode
SELECT 1;
@@ -616,12 +639,12 @@ PQsendQuery not allowed in pipeline mode
-- commit the implicit transaction block. The first command after a
-- sync will not be seen as belonging to a pipeline.
\startpipeline
-SET LOCAL statement_timeout='1h' \bind \g
-SHOW statement_timeout \bind \g
+SET LOCAL statement_timeout='1h' \bind \sendpipeline
+SHOW statement_timeout \bind \sendpipeline
\syncpipeline
-SHOW statement_timeout \bind \g
-SET LOCAL statement_timeout='2h' \bind \g
-SHOW statement_timeout \bind \g
+SHOW statement_timeout \bind \sendpipeline
+SET LOCAL statement_timeout='2h' \bind \sendpipeline
+SHOW statement_timeout \bind \sendpipeline
\endpipeline
WARNING: SET LOCAL can only be used in transaction blocks
statement_timeout
@@ -641,9 +664,9 @@ WARNING: SET LOCAL can only be used in transaction blocks
-- REINDEX CONCURRENTLY fails if not the first command in a pipeline.
\startpipeline
-SELECT $1 \bind 1 \g
-REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
?column?
----------
@@ -653,8 +676,8 @@ SELECT $1 \bind 2 \g
ERROR: REINDEX CONCURRENTLY cannot run inside a transaction block
-- REINDEX CONCURRENTLY works if it is the first command in a pipeline.
\startpipeline
-REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
?column?
----------
@@ -663,25 +686,25 @@ SELECT $1 \bind 2 \g
-- Subtransactions are not allowed in a pipeline.
\startpipeline
-SAVEPOINT a \bind \g
-SELECT $1 \bind 1 \g
-ROLLBACK TO SAVEPOINT a \bind \g
-SELECT $1 \bind 2 \g
+SAVEPOINT a \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
+ROLLBACK TO SAVEPOINT a \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
ERROR: SAVEPOINT can only be used in transaction blocks
-- LOCK fails as the first command in a pipeline, as not seen in an
-- implicit transaction block.
\startpipeline
-LOCK psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+LOCK psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
ERROR: LOCK TABLE can only be used in transaction blocks
-- LOCK succeeds as it is not the first command in a pipeline,
-- seen in an implicit transaction block.
\startpipeline
-SELECT $1 \bind 1 \g
-LOCK psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+LOCK psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
?column?
----------
@@ -695,12 +718,12 @@ SELECT $1 \bind 2 \g
-- VACUUM works as the first command in a pipeline.
\startpipeline
-VACUUM psql_pipeline \bind \g
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
-- VACUUM fails when not the first command in a pipeline.
\startpipeline
-SELECT 1 \bind \g
-VACUUM psql_pipeline \bind \g
+SELECT 1 \bind \sendpipeline
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
?column?
----------
@@ -710,9 +733,9 @@ VACUUM psql_pipeline \bind \g
ERROR: VACUUM cannot run inside a transaction block
-- VACUUM works after a \syncpipeline.
\startpipeline
-SELECT 1 \bind \g
+SELECT 1 \bind \sendpipeline
\syncpipeline
-VACUUM psql_pipeline \bind \g
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
?column?
----------
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 97d1be3aac3..b6ba947b812 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1079,6 +1079,7 @@ select \if false \\ (bogus \else \\ 42 \endif \\ forty_two;
\sf whole_line
\sv whole_line
\startpipeline
+ \sendpipeline
\syncpipeline
\t arg1
\T arg1
diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql
index 6517ebb71f8..256081dfd5f 100644
--- a/src/test/regress/sql/psql_pipeline.sql
+++ b/src/test/regress/sql/psql_pipeline.sql
@@ -6,23 +6,23 @@ CREATE TABLE psql_pipeline(a INTEGER PRIMARY KEY, s TEXT);
-- Single query
\startpipeline
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
-- Multiple queries
\startpipeline
-SELECT $1 \bind 'val1' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\endpipeline
-- Test \flush
\startpipeline
\flush
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\flush
-SELECT $1, $2 \bind 'val2' 'val3' \g
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\endpipeline
-- Send multiple syncs
@@ -30,12 +30,12 @@ SELECT $1, $2 \bind 'val2' 'val3' \g
\echo :PIPELINE_COMMAND_COUNT
\echo :PIPELINE_SYNC_COUNT
\echo :PIPELINE_RESULT_COUNT
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\syncpipeline
\syncpipeline
-SELECT $1, $2 \bind 'val2' 'val3' \g
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
\syncpipeline
-SELECT $1, $2 \bind 'val4' 'val5' \g
+SELECT $1, $2 \bind 'val4' 'val5' \sendpipeline
\echo :PIPELINE_COMMAND_COUNT
\echo :PIPELINE_SYNC_COUNT
\echo :PIPELINE_RESULT_COUNT
@@ -44,39 +44,39 @@ SELECT $1, $2 \bind 'val4' 'val5' \g
-- \startpipeline should not have any effect if already in a pipeline.
\startpipeline
\startpipeline
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
-- Convert an implicit transaction block to an explicit transaction block.
\startpipeline
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 2 \g
-ROLLBACK \bind \g
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 2 \sendpipeline
+ROLLBACK \bind \sendpipeline
\endpipeline
-- Multiple explicit transactions
\startpipeline
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-ROLLBACK \bind \g
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-COMMIT \bind \g
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+ROLLBACK \bind \sendpipeline
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+COMMIT \bind \sendpipeline
\endpipeline
-- COPY FROM STDIN
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\endpipeline
2 test2
\.
-- COPY FROM STDIN with \flushrequest + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\flushrequest
\getresults
3 test3
@@ -85,8 +85,8 @@ COPY psql_pipeline FROM STDIN \bind \g
-- COPY FROM STDIN with \syncpipeline + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-COPY psql_pipeline FROM STDIN \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+COPY psql_pipeline FROM STDIN \bind \sendpipeline
\syncpipeline
\getresults
4 test4
@@ -95,22 +95,22 @@ COPY psql_pipeline FROM STDIN \bind \g
-- COPY TO STDOUT
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\endpipeline
-- COPY TO STDOUT with \flushrequest + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\flushrequest
\getresults
\endpipeline
-- COPY TO STDOUT with \syncpipeline + \getresults
\startpipeline
-SELECT $1 \bind 'val1' \g
-copy psql_pipeline TO STDOUT \bind \g
+SELECT $1 \bind 'val1' \sendpipeline
+copy psql_pipeline TO STDOUT \bind \sendpipeline
\syncpipeline
\getresults
\endpipeline
@@ -120,22 +120,22 @@ copy psql_pipeline TO STDOUT \bind \g
SELECT $1 \parse ''
SELECT $1, $2 \parse ''
SELECT $2 \parse pipeline_1
-\bind_named '' 1 2 \g
-\bind_named pipeline_1 2 \g
+\bind_named '' 1 2 \sendpipeline
+\bind_named pipeline_1 2 \sendpipeline
\endpipeline
-- \getresults displays all results preceding a \flushrequest.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
\getresults
\endpipeline
-- \getresults displays all results preceding a \syncpipeline.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\syncpipeline
\getresults
\endpipeline
@@ -143,7 +143,7 @@ SELECT $1 \bind 2 \g
-- \getresults immediately returns if there is no result to fetch.
\startpipeline
\getresults
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
\flushrequest
\endpipeline
@@ -151,42 +151,42 @@ SELECT $1 \bind 2 \g
-- \getresults only fetches results preceding a \flushrequest.
\startpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
\endpipeline
-- \getresults only fetches results preceding a \syncpipeline.
\startpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\syncpipeline
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\getresults
\endpipeline
-- Use pipeline with chunked results for both \getresults and \endpipeline.
\startpipeline
\set FETCH_COUNT 10
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\flushrequest
\getresults
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
\unset FETCH_COUNT
-- \getresults with specific number of requested results.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
-SELECT $1 \bind 3 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
+SELECT $1 \bind 3 \sendpipeline
\echo :PIPELINE_SYNC_COUNT
\syncpipeline
\echo :PIPELINE_SYNC_COUNT
\echo :PIPELINE_RESULT_COUNT
\getresults 1
\echo :PIPELINE_RESULT_COUNT
-SELECT $1 \bind 4 \g
+SELECT $1 \bind 4 \sendpipeline
\getresults 3
\echo :PIPELINE_RESULT_COUNT
\endpipeline
@@ -195,7 +195,7 @@ SELECT $1 \bind 4 \g
\startpipeline
\syncpipeline
\syncpipeline
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\flushrequest
\getresults 2
\getresults 1
@@ -203,9 +203,9 @@ SELECT $1 \bind 1 \g
-- \getresults 0 should get all the results.
\startpipeline
-SELECT $1 \bind 1 \g
-SELECT $1 \bind 2 \g
-SELECT $1 \bind 3 \g
+SELECT $1 \bind 1 \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
+SELECT $1 \bind 3 \sendpipeline
\syncpipeline
\getresults 0
\endpipeline
@@ -221,36 +221,36 @@ SELECT $1 \bind 3 \g
-- pipeline usable.
\startpipeline
SELECT 1;
-SELECT $1 \bind 'val1' \g
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\syncpipeline
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
-- For an incorrect number of parameters, the pipeline is aborted and
-- the following queries will not be executed.
\startpipeline
-SELECT \bind 'val1' \g
-SELECT $1 \bind 'val1' \g
+SELECT \bind 'val1' \sendpipeline
+SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
-BEGIN \bind \g
-INSERT INTO psql_pipeline VALUES ($1) \bind 1 \g
-ROLLBACK \bind \g
+BEGIN \bind \sendpipeline
+INSERT INTO psql_pipeline VALUES ($1) \bind 1 \sendpipeline
+ROLLBACK \bind \sendpipeline
\endpipeline
ROLLBACK;
-- \watch sends a simple query, something not allowed within a pipeline.
\startpipeline
-SELECT \bind \g
+SELECT \bind \sendpipeline
\watch 1
\endpipeline
@@ -258,7 +258,7 @@ SELECT \bind \g
-- and the pipeline should still be usable.
\startpipeline
SELECT $1 \bind 1 \gdesc
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
-- \gset is not allowed in a pipeline, pipeline should still be usable.
@@ -267,54 +267,72 @@ SELECT $1 as i, $2 as j \parse ''
SELECT $1 as k, $2 as l \parse 'second'
\bind_named '' 1 2 \gset
\bind_named second 1 2 \gset pref02_ \echo :pref02_i :pref02_j
-\bind_named '' 1 2 \g
+\bind_named '' 1 2 \sendpipeline
\endpipeline
--- \gx is not allowed, pipeline should still be usable.
+-- \g and \gx are not allowed, pipeline should still be usable.
\startpipeline
+SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \g filename
+SELECT $1 \bind 1 \g |cat
+SELECT $1 \bind 1 \g (format=unaligned tuples_only=on)
SELECT $1 \bind 1 \gx
+SELECT $1 \bind 1 \gx filename
+SELECT $1 \bind 1 \gx |cat
+SELECT $1 \bind 1 \gx (format=unaligned tuples_only=on)
\reset
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
\endpipeline
-- \gx warning should be emitted in an aborted pipeline, with
-- pipeline still usable.
\startpipeline
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\flushrequest
\getresults
SELECT $1 \bind 1 \gx
\endpipeline
+-- \sendpipeline is not allowed outside of a pipeline
+\sendpipeline
+SELECT $1 \bind 1 \sendpipeline
+\reset
+
+-- \sendpipeline is not allowed if not preceded by \bind or \bind_named
+\startpipeline
+\sendpipeline
+SELECT 1 \sendpipeline
+\endpipeline
+
-- \gexec is not allowed, pipeline should still be usable.
\startpipeline
SELECT 'INSERT INTO psql_pipeline(a) SELECT generate_series(1, 10)' \parse 'insert_stmt'
\bind_named insert_stmt \gexec
-\bind_named insert_stmt \g
+\bind_named insert_stmt \sendpipeline
SELECT COUNT(*) FROM psql_pipeline \bind \g
\endpipeline
-- After an error, pipeline is aborted and requires \syncpipeline to be
-- reusable.
\startpipeline
-SELECT $1 \bind \g
-SELECT $1 \bind 1 \g
+SELECT $1 \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
\flushrequest
\getresults
-- Pipeline is aborted.
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
-- Sync allows pipeline to recover.
\syncpipeline
\getresults
-SELECT $1 \bind 1 \g
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\close a
\flushrequest
\getresults
@@ -322,10 +340,10 @@ SELECT $1 \parse a
-- In an aborted pipeline, \getresults 1 aborts commands one at a time.
\startpipeline
-SELECT $1 \bind \g
-SELECT $1 \bind 1 \g
+SELECT $1 \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
SELECT $1 \parse a
-\bind_named a 1 \g
+\bind_named a 1 \sendpipeline
\syncpipeline
\getresults 1
\getresults 1
@@ -337,10 +355,10 @@ SELECT $1 \parse a
-- Test chunked results with an aborted pipeline.
\startpipeline
\set FETCH_COUNT 10
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\flushrequest
\getresults
-SELECT $1 \bind \g
+SELECT $1 \bind \sendpipeline
\endpipeline
\unset FETCH_COUNT
@@ -356,7 +374,7 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
-SELECT 1 \bind \g
+SELECT 1 \bind \sendpipeline
SELECT 1;
SELECT 1;
\endpipeline
@@ -371,66 +389,66 @@ SELECT 1;
-- commit the implicit transaction block. The first command after a
-- sync will not be seen as belonging to a pipeline.
\startpipeline
-SET LOCAL statement_timeout='1h' \bind \g
-SHOW statement_timeout \bind \g
+SET LOCAL statement_timeout='1h' \bind \sendpipeline
+SHOW statement_timeout \bind \sendpipeline
\syncpipeline
-SHOW statement_timeout \bind \g
-SET LOCAL statement_timeout='2h' \bind \g
-SHOW statement_timeout \bind \g
+SHOW statement_timeout \bind \sendpipeline
+SET LOCAL statement_timeout='2h' \bind \sendpipeline
+SHOW statement_timeout \bind \sendpipeline
\endpipeline
-- REINDEX CONCURRENTLY fails if not the first command in a pipeline.
\startpipeline
-SELECT $1 \bind 1 \g
-REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
-- REINDEX CONCURRENTLY works if it is the first command in a pipeline.
\startpipeline
-REINDEX TABLE CONCURRENTLY psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+REINDEX TABLE CONCURRENTLY psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
-- Subtransactions are not allowed in a pipeline.
\startpipeline
-SAVEPOINT a \bind \g
-SELECT $1 \bind 1 \g
-ROLLBACK TO SAVEPOINT a \bind \g
-SELECT $1 \bind 2 \g
+SAVEPOINT a \bind \sendpipeline
+SELECT $1 \bind 1 \sendpipeline
+ROLLBACK TO SAVEPOINT a \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
-- LOCK fails as the first command in a pipeline, as not seen in an
-- implicit transaction block.
\startpipeline
-LOCK psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+LOCK psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
-- LOCK succeeds as it is not the first command in a pipeline,
-- seen in an implicit transaction block.
\startpipeline
-SELECT $1 \bind 1 \g
-LOCK psql_pipeline \bind \g
-SELECT $1 \bind 2 \g
+SELECT $1 \bind 1 \sendpipeline
+LOCK psql_pipeline \bind \sendpipeline
+SELECT $1 \bind 2 \sendpipeline
\endpipeline
-- VACUUM works as the first command in a pipeline.
\startpipeline
-VACUUM psql_pipeline \bind \g
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
-- VACUUM fails when not the first command in a pipeline.
\startpipeline
-SELECT 1 \bind \g
-VACUUM psql_pipeline \bind \g
+SELECT 1 \bind \sendpipeline
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
-- VACUUM works after a \syncpipeline.
\startpipeline
-SELECT 1 \bind \g
+SELECT 1 \bind \sendpipeline
\syncpipeline
-VACUUM psql_pipeline \bind \g
+VACUUM psql_pipeline \bind \sendpipeline
\endpipeline
-- Clean up
--
2.39.5 (Apple Git-154)
[application/octet-stream] v02-0002-psql-Allow-to-add-queries-in-an-ongoing-pipeline.patch (7.5K, ../../CAO6_Xqq4JEHRaGyC=r+dpi0Ejbp2P3aUo-KfR2NuipCvE-o_ZA@mail.gmail.com/3-v02-0002-psql-Allow-to-add-queries-in-an-ongoing-pipeline.patch)
download | inline diff:
From 2ebe091c9fcc5cd35bbbc02ece90645539330ebd Mon Sep 17 00:00:00 2001
From: Anthonin Bonnefoy <[email protected]>
Date: Wed, 5 Mar 2025 14:55:33 +0100
Subject: psql: Allow ';' to add queries in an ongoing pipeline
Currently, the only way to pipe queries in an ongoing pipeline is to
leverage psql meta-commands to create extended queries such as \bind,
\parse or \bind_named. This prevents using psql's pipeline on existing
scripts as it would require to convert all queries to use those
meta-commands.
This patch modifies ';' behaviour within an active pipeline and send all
queries as extended queries, allowing them to be piped in a pipeline.
---
doc/src/sgml/ref/psql-ref.sgml | 21 +++++----
src/bin/psql/command.c | 7 +++
src/bin/psql/common.c | 10 +++-
src/test/regress/expected/psql_pipeline.out | 52 +++++++++++++--------
src/test/regress/sql/psql_pipeline.sql | 24 ++++++----
5 files changed, 77 insertions(+), 37 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index cddf6e07531..2763486e268 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3698,14 +3698,19 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
</para>
<para>
- Pipeline mode requires the use of the extended query protocol. All
- queries need to be sent using the meta-commands
- <literal>\bind</literal>, <literal>\bind_named</literal>,
- <literal>\close</literal> or <literal>\parse</literal>. While a
- pipeline is ongoing, <literal>\sendpipeline</literal> will append the
- current query buffer to the pipeline. Other meta-commands like
- <literal>\g</literal>, <literal>\gx</literal> or <literal>\gdesc</literal>
- are not allowed in pipeline mode.
+ Pipeline mode requires the use of the extended query protocol. Queries
+ can be sent using the meta-commands <literal>\bind</literal>,
+ <literal>\bind_named</literal>, <literal>\close</literal> or
+ <literal>\parse</literal>. While a pipeline is ongoing,
+ <literal>\sendpipeline</literal> will append the current query
+ buffer to the pipeline. Other meta-commands like <literal>\g</literal>,
+ <literal>\gx</literal> or <literal>\gdesc</literal> are not allowed
+ in pipeline mode.
+ </para>
+
+ <para>
+ Queries can also be sent using <literal>;</literal>. While a pipeline is
+ ongoing, they will automatically be sent using extended query protocol.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 7670c20b29e..3e7eb086367 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3282,6 +3282,13 @@ exec_command_watch(PsqlScanState scan_state, bool active_branch,
int iter = 0;
int min_rows = 0;
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ {
+ pg_log_error("\\watch not allowed in pipeline mode");
+ clean_extended_state();
+ success = false;
+ }
+
/*
* Parse arguments. We allow either an unlabeled interval or
* "name=value", where name is from the set ('i', 'interval', 'c',
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index ed340a466f9..5249336bcf2 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1668,7 +1668,15 @@ ExecQueryAndProcessResults(const char *query,
}
break;
case PSQL_SEND_QUERY:
- success = PQsendQuery(pset.db, query);
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ {
+ success = PQsendQueryParams(pset.db, query,
+ 0, NULL, NULL, NULL, NULL, 0);
+ if (success)
+ pset.piped_commands++;
+ }
+ else
+ success = PQsendQuery(pset.db, query);
break;
}
diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out
index 53f2edfd986..f05429b36ac 100644
--- a/src/test/regress/expected/psql_pipeline.out
+++ b/src/test/regress/expected/psql_pipeline.out
@@ -387,24 +387,33 @@ SELECT $1 \bind 3 \sendpipeline
(1 row)
\endpipeline
+-- Queries can be pipelined ';'. They will be sent using extended protocol.
+\startpipeline
+SELECT 1;
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT 2;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ val1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
--
-- Pipeline errors
--
-- \endpipeline outside of pipeline should fail
\endpipeline
cannot send pipeline when not in pipeline mode
--- Query using simple protocol should not be sent and should leave the
--- pipeline usable.
-\startpipeline
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-SELECT $1 \bind 'val1' \sendpipeline
-\endpipeline
- ?column?
-----------
- val1
-(1 row)
-
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
@@ -425,6 +434,12 @@ SELECT \bind 'val1' \sendpipeline
SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
ERROR: bind message supplies 1 parameters, but prepared statement "" requires 0
+-- Using ';' with a parameter will trigger an incorrect parameter errors.
+\startpipeline
+SELECT $1;
+SELECT 1;
+\endpipeline
+ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
@@ -439,8 +454,7 @@ ROLLBACK;
\startpipeline
SELECT \bind \sendpipeline
\watch 1
-PQsendQuery not allowed in pipeline mode
-
+\watch not allowed in pipeline mode
\endpipeline
--
(1 row)
@@ -619,11 +633,11 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
SELECT 1 \bind \sendpipeline
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-PQsendQuery not allowed in pipeline mode
+\gdesc
+synchronous command execution functions are not allowed in pipeline mode
+\gdesc
+synchronous command execution functions are not allowed in pipeline mode
+synchronous command execution functions are not allowed in pipeline mode
\endpipeline
?column?
----------
diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql
index 256081dfd5f..76c0ece01af 100644
--- a/src/test/regress/sql/psql_pipeline.sql
+++ b/src/test/regress/sql/psql_pipeline.sql
@@ -210,6 +210,13 @@ SELECT $1 \bind 3 \sendpipeline
\getresults 0
\endpipeline
+-- Queries can be pipelined ';'. They will be sent using extended protocol.
+\startpipeline
+SELECT 1;
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT 2;
+\endpipeline
+
--
-- Pipeline errors
--
@@ -217,13 +224,6 @@ SELECT $1 \bind 3 \sendpipeline
-- \endpipeline outside of pipeline should fail
\endpipeline
--- Query using simple protocol should not be sent and should leave the
--- pipeline usable.
-\startpipeline
-SELECT 1;
-SELECT $1 \bind 'val1' \sendpipeline
-\endpipeline
-
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
@@ -239,6 +239,12 @@ SELECT \bind 'val1' \sendpipeline
SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
+-- Using ';' with a parameter will trigger an incorrect parameter errors.
+\startpipeline
+SELECT $1;
+SELECT 1;
+\endpipeline
+
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
@@ -375,8 +381,8 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
SELECT 1 \bind \sendpipeline
-SELECT 1;
-SELECT 1;
+\gdesc
+\gdesc
\endpipeline
--
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
@ 2025-03-17 10:19 ` Michael Paquier <[email protected]>
2 siblings, 0 replies; 50+ messages in thread
From: Michael Paquier @ 2025-03-17 10:19 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Mon, Mar 17, 2025 at 10:50:50AM +0100, Anthonin Bonnefoy wrote:
> 0001: This introduces the \sendpipeline meta-command and forbid \g in
> a pipeline. This is to fix the formatting options of \g that are not
> supported in a pipeline.
>
> 0002: Allows ';' to send a query using extended protocol when within a
> pipeline by using PQsendQueryParams with 0 parameters. It is not
> possible to send parameters with extended protocol this way and
> everything will be propagated through the query string, similar to a
> simple query.
Thanks for sending a new patch set. I was planning to look at the
situation tomorrow, and you have beaten me to it.
The split makes sense, and I'm OK with 0001. 0002 is going to require
a much closer lookup.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
@ 2025-03-18 00:50 ` Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2 siblings, 1 reply; 50+ messages in thread
From: Michael Paquier @ 2025-03-18 00:50 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Mon, Mar 17, 2025 at 10:50:50AM +0100, Anthonin Bonnefoy wrote:
> 0001: This introduces the \sendpipeline meta-command and forbid \g in
> a pipeline. This is to fix the formatting options of \g that are not
> supported in a pipeline.
- count
--------
- 4
-(1 row)
This removal done in the regression tests was not intentional.
I have done some reordering of the code around the new meta-command so
as things are ordered alphabetically, and applied the result.
> 0002: Allows ';' to send a query using extended protocol when within a
> pipeline by using PQsendQueryParams with 0 parameters. It is not
> possible to send parameters with extended protocol this way and
> everything will be propagated through the query string, similar to a
> simple query.
I like the simplicity of what you are doing here, relying on
PSQL_SEND_QUERY being the default so as we use PQsendQueryParams()
with no parameters rather than PQsendQuery() when the pipeline mode is
not off.
How about adding a check on PIPELINE_COMMAND_COUNT when sending a
query through this path? Should we check for more scenarios with
syncs and flushes as well when sending these queries?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
@ 2025-03-18 08:55 ` Anthonin Bonnefoy <[email protected]>
2025-03-18 09:27 ` Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-19 04:49 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
0 siblings, 2 replies; 50+ messages in thread
From: Anthonin Bonnefoy @ 2025-03-18 08:55 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Mar 18, 2025 at 1:50 AM Michael Paquier <[email protected]> wrote:
> - count
> --------
> - 4
> -(1 row)
>
> This removal done in the regression tests was not intentional.
Yes, thanks for fixing that.
> How about adding a check on PIPELINE_COMMAND_COUNT when sending a
> query through this path? Should we check for more scenarios with
> syncs and flushes as well when sending these queries?
I've added additional tests when piping queries with ';':
- I've reused the same scenario with \sendpipeline: single query,
multiple queries, flushes, syncs, using COPY...
- Using ';' will replace the unnamed prepared statement. It's a bit
different from expected as a simple query will delete the unnamed
prepared statement.
- Sending an extended query prepared with \bind using a ';' on a
newline, though this is not specific to pipelining. The scanned
semicolon triggers the call to SendQuery, processing the buffered
extended query. It's a bit unusual but that's the current behaviour.
Attachments:
[application/octet-stream] v03-0001-psql-Allow-to-add-queries-in-an-ongoing-pipeline.patch (13.5K, ../../CAO6_Xqo=fNh0KCRZG7T18OZfjiabeL-t4C6mkXsz-3dAZ1YwTA@mail.gmail.com/2-v03-0001-psql-Allow-to-add-queries-in-an-ongoing-pipeline.patch)
download | inline diff:
From ae9ce7e44c1e2502429920c5d305e1ffcd30b1a5 Mon Sep 17 00:00:00 2001
From: Anthonin Bonnefoy <[email protected]>
Date: Wed, 5 Mar 2025 14:55:33 +0100
Subject: psql: Allow ';' to add queries in an ongoing pipeline
Currently, the only way to pipe queries in an ongoing pipeline is to
leverage psql meta-commands to create extended queries such as \bind,
\parse or \bind_named. This prevents using psql's pipeline on existing
scripts as it would require to convert all queries to use those
meta-commands.
This patch modifies ';' behaviour within an active pipeline and send all
queries as extended queries, allowing them to be piped in a pipeline.
---
doc/src/sgml/ref/psql-ref.sgml | 21 +-
src/bin/psql/command.c | 7 +
src/bin/psql/common.c | 10 +-
src/test/regress/expected/psql_pipeline.out | 292 ++++++++++++++++++--
src/test/regress/sql/psql_pipeline.sql | 139 +++++++++-
5 files changed, 429 insertions(+), 40 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index cddf6e07531..2763486e268 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3698,14 +3698,19 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
</para>
<para>
- Pipeline mode requires the use of the extended query protocol. All
- queries need to be sent using the meta-commands
- <literal>\bind</literal>, <literal>\bind_named</literal>,
- <literal>\close</literal> or <literal>\parse</literal>. While a
- pipeline is ongoing, <literal>\sendpipeline</literal> will append the
- current query buffer to the pipeline. Other meta-commands like
- <literal>\g</literal>, <literal>\gx</literal> or <literal>\gdesc</literal>
- are not allowed in pipeline mode.
+ Pipeline mode requires the use of the extended query protocol. Queries
+ can be sent using the meta-commands <literal>\bind</literal>,
+ <literal>\bind_named</literal>, <literal>\close</literal> or
+ <literal>\parse</literal>. While a pipeline is ongoing,
+ <literal>\sendpipeline</literal> will append the current query
+ buffer to the pipeline. Other meta-commands like <literal>\g</literal>,
+ <literal>\gx</literal> or <literal>\gdesc</literal> are not allowed
+ in pipeline mode.
+ </para>
+
+ <para>
+ Queries can also be sent using <literal>;</literal>. While a pipeline is
+ ongoing, they will automatically be sent using extended query protocol.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a87ff7e4597..bbe337780ff 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3282,6 +3282,13 @@ exec_command_watch(PsqlScanState scan_state, bool active_branch,
int iter = 0;
int min_rows = 0;
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ {
+ pg_log_error("\\watch not allowed in pipeline mode");
+ clean_extended_state();
+ success = false;
+ }
+
/*
* Parse arguments. We allow either an unlabeled interval or
* "name=value", where name is from the set ('i', 'interval', 'c',
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index ed340a466f9..5249336bcf2 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1668,7 +1668,15 @@ ExecQueryAndProcessResults(const char *query,
}
break;
case PSQL_SEND_QUERY:
- success = PQsendQuery(pset.db, query);
+ if (PQpipelineStatus(pset.db) != PQ_PIPELINE_OFF)
+ {
+ success = PQsendQueryParams(pset.db, query,
+ 0, NULL, NULL, NULL, NULL, 0);
+ if (success)
+ pset.piped_commands++;
+ }
+ else
+ success = PQsendQuery(pset.db, query);
break;
}
diff --git a/src/test/regress/expected/psql_pipeline.out b/src/test/regress/expected/psql_pipeline.out
index 68e3c19ea05..7dddf26a9fd 100644
--- a/src/test/regress/expected/psql_pipeline.out
+++ b/src/test/regress/expected/psql_pipeline.out
@@ -386,6 +386,262 @@ SELECT $1 \bind 3 \sendpipeline
3
(1 row)
+\endpipeline
+--
+-- Tests pipelining queries with ';'
+--
+-- Single query sent with ';'
+\startpipeline
+SELECT 1;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Multiple queries sent with ';'
+\startpipeline
+SELECT 1;
+SELECT 2;
+SELECT 3;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+ ?column?
+----------
+ 3
+(1 row)
+
+-- Multiple queries on the same line can be piped with ';'
+\startpipeline
+SELECT 1; SELECT 2; SELECT 3
+;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+ ?column?
+----------
+ 3
+(1 row)
+
+-- Test \flush with queries piped with ';'
+\startpipeline
+\flush
+SELECT 1;
+\flush
+SELECT 2;
+SELECT 3;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+ ?column?
+----------
+ 3
+(1 row)
+
+-- Send multiple syncs with queries piped with ';'
+\startpipeline
+\echo :PIPELINE_COMMAND_COUNT
+0
+\echo :PIPELINE_SYNC_COUNT
+0
+\echo :PIPELINE_RESULT_COUNT
+0
+SELECT 1;
+\syncpipeline
+\syncpipeline
+SELECT 2;
+\syncpipeline
+SELECT 3;
+\echo :PIPELINE_COMMAND_COUNT
+1
+\echo :PIPELINE_SYNC_COUNT
+3
+\echo :PIPELINE_RESULT_COUNT
+2
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+ ?column?
+----------
+ 3
+(1 row)
+
+-- Mix queries piped with ';' and \sendpipeline
+\startpipeline
+SELECT 1;
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT 2;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ val1
+(1 row)
+
+ ?column? | ?column?
+----------+----------
+ val2 | val3
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+-- Piping a query with ';' will replace the unnamed prepared statement
+\startpipeline
+SELECT $1 \parse ''
+SELECT 1;
+\bind_named ''
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+-- An extended query can be piped by a ';' after a newline
+\startpipeline
+SELECT $1 \bind 1
+;
+SELECT 2;
+\endpipeline
+ ?column?
+----------
+ 1
+(1 row)
+
+ ?column?
+----------
+ 2
+(1 row)
+
+-- COPY FROM STDIN, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\endpipeline
+ ?column?
+----------
+ val1
+(1 row)
+
+-- COPY FROM STDIN with \flushrequest + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\flushrequest
+\getresults
+ ?column?
+----------
+ val1
+(1 row)
+
+message type 0x5a arrived from server while idle
+\endpipeline
+-- COPY FROM STDIN with \syncpipeline + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\syncpipeline
+\getresults
+ ?column?
+----------
+ val1
+(1 row)
+
+\endpipeline
+-- COPY TO STDOUT, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\endpipeline
+ ?column?
+----------
+ val1
+(1 row)
+
+1 \N
+2 test2
+3 test3
+4 test4
+20 test2
+30 test3
+40 test4
+-- COPY TO STDOUT with \flushrequest + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\flushrequest
+\getresults
+ ?column?
+----------
+ val1
+(1 row)
+
+1 \N
+2 test2
+3 test3
+4 test4
+20 test2
+30 test3
+40 test4
+\endpipeline
+-- COPY TO STDOUT with \syncpipeline + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\syncpipeline
+\getresults
+ ?column?
+----------
+ val1
+(1 row)
+
+1 \N
+2 test2
+3 test3
+4 test4
+20 test2
+30 test3
+40 test4
\endpipeline
--
-- Pipeline errors
@@ -393,18 +649,6 @@ SELECT $1 \bind 3 \sendpipeline
-- \endpipeline outside of pipeline should fail
\endpipeline
cannot send pipeline when not in pipeline mode
--- Query using simple protocol should not be sent and should leave the
--- pipeline usable.
-\startpipeline
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-SELECT $1 \bind 'val1' \sendpipeline
-\endpipeline
- ?column?
-----------
- val1
-(1 row)
-
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
@@ -425,6 +669,13 @@ SELECT \bind 'val1' \sendpipeline
SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
ERROR: bind message supplies 1 parameters, but prepared statement "" requires 0
+-- Using ';' with a parameter will trigger an incorrect parameter error and
+-- abort the pipeline
+\startpipeline
+SELECT $1;
+SELECT 1;
+\endpipeline
+ERROR: bind message supplies 0 parameters, but prepared statement "" requires 1
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
@@ -435,12 +686,11 @@ ROLLBACK \bind \sendpipeline
ERROR: duplicate key value violates unique constraint "psql_pipeline_pkey"
DETAIL: Key (a)=(1) already exists.
ROLLBACK;
--- \watch sends a simple query, something not allowed within a pipeline.
+-- \watch is not allowed within a pipeline.
\startpipeline
SELECT \bind \sendpipeline
\watch 1
-PQsendQuery not allowed in pipeline mode
-
+\watch not allowed in pipeline mode
\endpipeline
--
(1 row)
@@ -530,7 +780,7 @@ SELECT COUNT(*) FROM psql_pipeline \bind \sendpipeline
count
-------
- 4
+ 7
(1 row)
-- After an error, pipeline is aborted and requires \syncpipeline to be
@@ -617,11 +867,11 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
SELECT 1 \bind \sendpipeline
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-SELECT 1;
-PQsendQuery not allowed in pipeline mode
-PQsendQuery not allowed in pipeline mode
+\gdesc
+synchronous command execution functions are not allowed in pipeline mode
+\gdesc
+synchronous command execution functions are not allowed in pipeline mode
+synchronous command execution functions are not allowed in pipeline mode
\endpipeline
?column?
----------
diff --git a/src/test/regress/sql/psql_pipeline.sql b/src/test/regress/sql/psql_pipeline.sql
index e4d7e614af3..6f76adcba82 100644
--- a/src/test/regress/sql/psql_pipeline.sql
+++ b/src/test/regress/sql/psql_pipeline.sql
@@ -210,6 +210,125 @@ SELECT $1 \bind 3 \sendpipeline
\getresults 0
\endpipeline
+--
+-- Tests pipelining queries with ';'
+--
+
+-- Single query sent with ';'
+\startpipeline
+SELECT 1;
+\endpipeline
+
+-- Multiple queries sent with ';'
+\startpipeline
+SELECT 1;
+SELECT 2;
+SELECT 3;
+\endpipeline
+
+-- Multiple queries on the same line can be piped with ';'
+\startpipeline
+SELECT 1; SELECT 2; SELECT 3
+;
+\endpipeline
+
+-- Test \flush with queries piped with ';'
+\startpipeline
+\flush
+SELECT 1;
+\flush
+SELECT 2;
+SELECT 3;
+\endpipeline
+
+-- Send multiple syncs with queries piped with ';'
+\startpipeline
+\echo :PIPELINE_COMMAND_COUNT
+\echo :PIPELINE_SYNC_COUNT
+\echo :PIPELINE_RESULT_COUNT
+SELECT 1;
+\syncpipeline
+\syncpipeline
+SELECT 2;
+\syncpipeline
+SELECT 3;
+\echo :PIPELINE_COMMAND_COUNT
+\echo :PIPELINE_SYNC_COUNT
+\echo :PIPELINE_RESULT_COUNT
+\endpipeline
+
+-- Mix queries piped with ';' and \sendpipeline
+\startpipeline
+SELECT 1;
+SELECT $1 \bind 'val1' \sendpipeline
+SELECT $1, $2 \bind 'val2' 'val3' \sendpipeline
+SELECT 2;
+\endpipeline
+
+-- Piping a query with ';' will replace the unnamed prepared statement
+\startpipeline
+SELECT $1 \parse ''
+SELECT 1;
+\bind_named ''
+\endpipeline
+
+-- An extended query can be piped by a ';' after a newline
+\startpipeline
+SELECT $1 \bind 1
+;
+SELECT 2;
+\endpipeline
+
+-- COPY FROM STDIN, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\endpipeline
+20 test2
+\.
+
+-- COPY FROM STDIN with \flushrequest + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\flushrequest
+\getresults
+30 test3
+\.
+\endpipeline
+
+-- COPY FROM STDIN with \syncpipeline + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+COPY psql_pipeline FROM STDIN;
+\syncpipeline
+\getresults
+40 test4
+\.
+\endpipeline
+
+-- COPY TO STDOUT, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\endpipeline
+
+-- COPY TO STDOUT with \flushrequest + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\flushrequest
+\getresults
+\endpipeline
+
+-- COPY TO STDOUT with \syncpipeline + \getresults, using ';'
+\startpipeline
+SELECT 'val1';
+copy psql_pipeline TO STDOUT;
+\syncpipeline
+\getresults
+\endpipeline
+
--
-- Pipeline errors
--
@@ -217,13 +336,6 @@ SELECT $1 \bind 3 \sendpipeline
-- \endpipeline outside of pipeline should fail
\endpipeline
--- Query using simple protocol should not be sent and should leave the
--- pipeline usable.
-\startpipeline
-SELECT 1;
-SELECT $1 \bind 'val1' \sendpipeline
-\endpipeline
-
-- After an aborted pipeline, commands after a \syncpipeline should be
-- displayed.
\startpipeline
@@ -239,6 +351,13 @@ SELECT \bind 'val1' \sendpipeline
SELECT $1 \bind 'val1' \sendpipeline
\endpipeline
+-- Using ';' with a parameter will trigger an incorrect parameter error and
+-- abort the pipeline
+\startpipeline
+SELECT $1;
+SELECT 1;
+\endpipeline
+
-- An explicit transaction with an error needs to be rollbacked after
-- the pipeline.
\startpipeline
@@ -248,7 +367,7 @@ ROLLBACK \bind \sendpipeline
\endpipeline
ROLLBACK;
--- \watch sends a simple query, something not allowed within a pipeline.
+-- \watch is not allowed within a pipeline.
\startpipeline
SELECT \bind \sendpipeline
\watch 1
@@ -372,8 +491,8 @@ select 1;
-- Error messages accumulate and are repeated.
\startpipeline
SELECT 1 \bind \sendpipeline
-SELECT 1;
-SELECT 1;
+\gdesc
+\gdesc
\endpipeline
--
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
@ 2025-03-18 09:27 ` Jelte Fennema-Nio <[email protected]>
2025-03-19 02:28 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
1 sibling, 1 reply; 50+ messages in thread
From: Jelte Fennema-Nio @ 2025-03-18 09:27 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Michael Paquier <[email protected]>; Daniel Verite <[email protected]>; pgsql-hackers
On Tue, 18 Mar 2025 at 09:55, Anthonin Bonnefoy
<[email protected]> wrote:
> I've added additional tests when piping queries with ';':
> - I've reused the same scenario with \sendpipeline: single query,
> multiple queries, flushes, syncs, using COPY...
> - Using ';' will replace the unnamed prepared statement. It's a bit
> different from expected as a simple query will delete the unnamed
> prepared statement.
> - Sending an extended query prepared with \bind using a ';' on a
> newline, though this is not specific to pipelining. The scanned
> semicolon triggers the call to SendQuery, processing the buffered
> extended query. It's a bit unusual but that's the current behaviour.
One thing that comes to mind that I think would be quite useful and
pretty easy to implement if we have this functionality within a
pipeline: An \extended command. That puts psql in "extended protocol
mode" (without enabling pipelining). In "extended protocol mode" all
queries would automatically be sent using PQsendQueryParams. That
would remove the need to use \bind anymore outside of a pipeline
either.
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 09:27 ` Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
@ 2025-03-19 02:28 ` Michael Paquier <[email protected]>
2025-03-19 13:05 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Michael Paquier @ 2025-03-19 02:28 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Daniel Verite <[email protected]>; pgsql-hackers
On Tue, Mar 18, 2025 at 10:27:38AM +0100, Jelte Fennema-Nio wrote:
> One thing that comes to mind that I think would be quite useful and
> pretty easy to implement if we have this functionality within a
> pipeline: An \extended command. That puts psql in "extended protocol
> mode" (without enabling pipelining). In "extended protocol mode" all
> queries would automatically be sent using PQsendQueryParams. That
> would remove the need to use \bind anymore outside of a pipeline
> either.
How does that help when passing parameter values? \bind is here to be
able to pass down parameter values to queries that are prepared, so we
cannot bypass it as the parameter values need to be passed to the
\bind meta-command itself.
Perhaps an \extended command that behaves outside a pipeline makes
sense to force the use of queries without parameters to use the
extended mode, but I cannot get much excited about the concept knowing
all the meta-commands we have now (not talking about the pipeline
part, which is different, as we can treat queries in batches).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 09:27 ` Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-19 02:28 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
@ 2025-03-19 13:05 ` Daniel Verite <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Daniel Verite @ 2025-03-19 13:05 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Anthonin Bonnefoy <[email protected]>; pgsql-hackers
Michael Paquier wrote:
> Perhaps an \extended command that behaves outside a pipeline makes
> sense to force the use of queries without parameters to use the
> extended mode, but I cannot get much excited about the concept knowing
> all the meta-commands we have now (not talking about the pipeline
> part, which is different, as we can treat queries in batches).
When psql started supporting the extended query protocol, the question
of enabling it more globally rather than query-by-query was discussed
a bit [1]. The idea was to switch to it with a setting or a variable
rather than a metacommand.
Some pros and cons were mentioned in the thread, but on the whole
it was not convincing enough to get implemented.
[1]
https://www.postgresql.org/message-id/e8dd1cd5-0e04-3598-0518-a605159fe314%40enterprisedb.com
Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
@ 2025-03-19 04:49 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 50+ messages in thread
From: Michael Paquier @ 2025-03-19 04:49 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Mar 18, 2025 at 09:55:21AM +0100, Anthonin Bonnefoy wrote:
> I've added additional tests when piping queries with ';':
> - I've reused the same scenario with \sendpipeline: single query,
> multiple queries, flushes, syncs, using COPY...
> - Using ';' will replace the unnamed prepared statement. It's a bit
> different from expected as a simple query will delete the unnamed
> prepared statement.
> - Sending an extended query prepared with \bind using a ';' on a
> newline, though this is not specific to pipelining. The scanned
> semicolon triggers the call to SendQuery, processing the buffered
> extended query. It's a bit unusual but that's the current behaviour.
The tests could be much more organized, particularly for the "sinple"
and "multiple" and COPY cases, rather than being treated as two
different groups at different locations of psql_pipeline.sql. I've
spent some time reorganizing all that.
A second thing that was a bit itchy is the use of ";" for what's a
semicolon, and we use this term in the psql docs to refer to queries
terminated by that. The whole paragraph could be simplified a bit
more, mentioning that everything in a pipeline uses the extended
protocol, while \bind & co are more like options. The description of
PIPELINE_COMMAND_COUNT could be simpler, and the part about the
pending results can be more general now so I've removed it.
With all that set, I've applied the patch. If you have more
suggestions, please feel free to mention them.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
@ 2025-03-18 09:36 ` Daniel Verite <[email protected]>
2025-03-18 23:56 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2 siblings, 1 reply; 50+ messages in thread
From: Daniel Verite @ 2025-03-18 09:36 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Michael Paquier <[email protected]>; pgsql-hackers
Anthonin Bonnefoy wrote:
> 0002: Allows ';' to send a query using extended protocol when within a
> pipeline by using PQsendQueryParams
It's a nice improvement!
> with 0 parameters. It is not
> possible to send parameters with extended protocol this way and
> everything will be propagated through the query string, similar to a
> simple query.
It's actually possible to use parameters
\startpipeline
\bind 'foo'
select $1;
\endpipeline
?column?
----------
foo
(1 row)
I suspect there's a misunderstanding that \bind can only be placed
after the query, because it's always written like that in the regression
tests, and in the documentation.
But that's just a notational preference.
Best regards,
--
Daniel Vérité
https://postgresql.verite.pro/
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 09:36 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
@ 2025-03-18 23:56 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Michael Paquier @ 2025-03-18 23:56 UTC (permalink / raw)
To: Daniel Verite <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Mar 18, 2025 at 10:36:28AM +0100, Daniel Verite wrote:
> It's actually possible to use parameters
>
> \startpipeline
> \bind 'foo'
> select $1;
> \endpipeline
>
> ?column?
> ----------
> foo
> (1 row)
>
> I suspect there's a misunderstanding that \bind can only be placed
> after the query, because it's always written like that in the regression
> tests, and in the documentation.
> But that's just a notational preference.
Nice trick, unrelated to pipelines. I don't think that we have
anything testing this specific pattern for \bind. At quick glance all
our test use \bind at the end of a query string. Perhaps we should?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
@ 2025-04-16 14:31 a.kozhemyakin <[email protected]>
2025-04-16 16:18 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: a.kozhemyakin @ 2025-04-16 14:31 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Daniel Verite <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
Hello,
After commit 2cce0fe on master
When executing query:
psql postgres <<EOF
CREATE TABLE psql_pipeline();
\startpipeline
COPY psql_pipeline FROM STDIN;
SELECT 'val1';
\syncpipeline
\getresults
EOF
ERROR: unexpected message type 0x50 during COPY from stdin
CONTEXT: COPY psql_pipeline, line 1
Pipeline aborted, command did not run
psql: common.c:1510: discardAbortedPipelineResults: Assertion `res ==
((void *)0) || result_status == PGRES_PIPELINE_ABORTED' failed.
Aborted (core dumped)
The psql crashes with the stack trace:
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6,
threadid=<optimized out>) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=<optimized out>) at
./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6)
at ./nptl/pthread_kill.c:89
#3 0x0000760edd24527e in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#4 0x0000760edd2288ff in __GI_abort () at ./stdlib/abort.c:79
#5 0x0000760edd22881b in __assert_fail_base (fmt=0x760edd3d01e8
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x5ba46ab79850 "res == ((void *)0) ||
result_status == PGRES_PIPELINE_ABORTED", file=file@entry=0x5ba46ab6fcad
"common.c",
line=line@entry=1510, function=function@entry=0x5ba46ab9c780
<__PRETTY_FUNCTION__.3> "discardAbortedPipelineResults") at
./assert/assert.c:96
#6 0x0000760edd23b517 in __assert_fail
(assertion=assertion@entry=0x5ba46ab79850 "res == ((void *)0) ||
result_status == PGRES_PIPELINE_ABORTED",
file=file@entry=0x5ba46ab6fcad "common.c", line=line@entry=1510,
function=function@entry=0x5ba46ab9c780 <__PRETTY_FUNCTION__.3>
"discardAbortedPipelineResults") at ./assert/assert.c:105
#7 0x00005ba46ab2bd40 in discardAbortedPipelineResults () at common.c:1510
#8 ExecQueryAndProcessResults (query=query@entry=0x5ba4a2ec1e10 "SELECT
'val1';", elapsed_msec=elapsed_msec@entry=0x7ffeb07262a8,
svpt_gone_p=svpt_gone_p@entry=0x7ffeb07262a7,
is_watch=is_watch@entry=false, min_rows=min_rows@entry=0,
opt=opt@entry=0x0, printQueryFout=0x0)
at common.c:1811
#9 0x00005ba46ab2983f in SendQuery (query=0x5ba4a2ec1e10 "SELECT
'val1';") at common.c:1212
#10 0x00005ba46ab3f66a in MainLoop (source=source@entry=0x760edd4038e0
<_IO_2_1_stdin_>) at mainloop.c:515
#11 0x00005ba46ab23f2a in process_file (filename=0x0,
use_relative_path=use_relative_path@entry=false) at command.c:4870
#12 0x00005ba46ab1e9d9 in main (argc=<optimized out>,
argv=0x7ffeb07269d8) at startup.c:420
06.03.2025 11:20, Michael Paquier пишет:
> On Wed, Mar 05, 2025 at 03:25:12PM +0100, Daniel Verite wrote:
>> Anthonin Bonnefoy wrote:
>>> I do see the idea to make it easier to convert existing scripts into
>>> using pipelining. The main focus of the initial implementation was
>>> more on protocol regression tests with psql, so that's not necessarily
>>> something I had in mind.
>> Understood. Yet pipelining can accelerate considerably certain scripts
>> when client-server latency is an issue. We should expect end users to
>> benefit from it too.
> That was not a test case we had in mind originally here, but if it is
> possible to keep the implementation simple while supporting your
> demand, well, let's do it. If it's not that straight-forward, let's
> use the new meta-command, forbidding \g and \gx based on your
> arguments from upthread.
> --
> Michael
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-04-16 14:31 Re: Add Pipelining support in psql a.kozhemyakin <[email protected]>
@ 2025-04-16 16:18 ` Michael Paquier <[email protected]>
2025-04-22 12:37 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
0 siblings, 1 reply; 50+ messages in thread
From: Michael Paquier @ 2025-04-16 16:18 UTC (permalink / raw)
To: a.kozhemyakin <[email protected]>; +Cc: Daniel Verite <[email protected]>; Anthonin Bonnefoy <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Wed, Apr 16, 2025 at 09:31:59PM +0700, a.kozhemyakin wrote:
> After commit 2cce0fe on master
>
> ERROR: unexpected message type 0x50 during COPY from stdin
> CONTEXT: COPY psql_pipeline, line 1
> Pipeline aborted, command did not run
> psql: common.c:1510: discardAbortedPipelineResults: Assertion `res == ((void
> *)0) || result_status == PGRES_PIPELINE_ABORTED' failed.
> Aborted (core dumped)
Reproduced here, thanks for the report. I'll look at that at the
beginning of next week, adding an open item for now.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 50+ messages in thread
* Re: Add Pipelining support in psql
2025-04-16 14:31 Re: Add Pipelining support in psql a.kozhemyakin <[email protected]>
2025-04-16 16:18 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
@ 2025-04-22 12:37 ` Anthonin Bonnefoy <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Anthonin Bonnefoy @ 2025-04-22 12:37 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: a.kozhemyakin <[email protected]>; Daniel Verite <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Apr 22, 2025 at 2:06 AM Michael Paquier <[email protected]> wrote:
> I am wondering if we could not be smarter with the handling of
> the counters, but I really doubt that there is much more we can do
> under a PGRES_FATAL_ERROR.
One thing that bothers me is that the reported error is silently
discarded within discardAbortedPipelineResults.
psql -f bug_assertion.sql
psql:bug_assertion.sql:7: ERROR: unexpected message type 0x50 during
COPY from stdin
CONTEXT: COPY psql_pipeline, line 1
psql:bug_assertion.sql:7: Pipeline aborted, command did not run
This should ideally report the "FATAL: terminating connection because
protocol synchronization was lost" sent by the backend process.
Also, we still have a triggered assertion failure with the following:
CREATE TABLE psql_pipeline(a text);
\startpipeline
COPY psql_pipeline FROM STDIN;
SELECT 'val1';
\syncpipeline
\endpipeline
...
Assertion failed: (pset.piped_syncs == 0), function
ExecQueryAndProcessResults, file common.c, line 2153.
A possible alternative could be to abort discardAbortedPipelineResults
when we encounter a res != NULL + FATAL error and let the outer loop
handle it. As you said, the pipeline flow is borked so there's not
much to salvage. The outer loop would read and print all error
messages until the closed connection is detected. Then,
CheckConnection will reset the connection which will reset the
pipeline state.
While testing this change, I was initially looking for the "FATAL:
terminating connection because protocol synchronization was lost"
message in the tests. However, this was failing on Windows[1] as the
FATAL message wasn't reported on stderr. I'm not sure why yet.
[1]: https://cirrus-ci.com/task/5051031505076224?logs=check_world#L240-L246
Attachments:
[application/octet-stream] v02-0001-PATCH-psql-Fix-assertion-failure-with-pipeline-m.patch (3.7K, ../../CAO6_XqoQaZVHEw1dFgvOxwectqsNo4QE1tQ6rLD-YUzFpT_+3g@mail.gmail.com/2-v02-0001-PATCH-psql-Fix-assertion-failure-with-pipeline-m.patch)
download | inline diff:
From 5d37f2616c82c6525d656149d383ef01a6d7518c Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 21 Apr 2025 15:17:43 +0900
Subject: [PATCH] psql: Fix assertion failure with pipeline mode
---
src/bin/psql/common.c | 17 ++++++++++++
src/bin/psql/t/001_basic.pl | 55 ++++++++++++++++++++++++++++++++++---
2 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 21d660a8961..0aab02ee32e 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1478,6 +1478,23 @@ discardAbortedPipelineResults(void)
*/
return res;
}
+ else if (res != NULL && result_status == PGRES_FATAL_ERROR)
+ {
+ /*
+ * We have a fatal error sent by the backend and we can't recover
+ * from this state. Instead, return the last fatal error and let
+ * the outer loop handle it.
+ */
+ PGresult *fatal_res PG_USED_FOR_ASSERTS_ONLY;
+
+ /*
+ * Fetch result to consume the end of the current query being
+ * processed.
+ */
+ fatal_res = PQgetResult(pset.db);
+ Assert(fatal_res == NULL);
+ return res;
+ }
else if (res == NULL)
{
/* A query was processed, decrement the counters */
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 739cb439708..8d258c00c5e 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -34,11 +34,13 @@ sub psql_fails_like
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
- my ($node, $sql, $expected_stderr, $test_name) = @_;
+ my ($node, $sql, $expected_stderr, $test_name, $replication) = @_;
+
+ # Use the context of a WAL sender, if requested by the caller.
+ $replication = '' unless defined($replication);
- # Use the context of a WAL sender, some of the tests rely on that.
my ($ret, $stdout, $stderr) =
- $node->psql('postgres', $sql, replication => 'database');
+ $node->psql('postgres', $sql, replication => $replication);
isnt($ret, 0, "$test_name: exit code not 0");
like($stderr, $expected_stderr, "$test_name: matches");
@@ -79,7 +81,8 @@ psql_fails_like(
$node,
'START_REPLICATION 0/0',
qr/unexpected PQresultStatus: 8$/,
- 'handling of unexpected PQresultStatus');
+ 'handling of unexpected PQresultStatus',
+ 'database');
# test \timing
psql_like(
@@ -481,4 +484,48 @@ psql_like($node, "copy (values ('foo'),('bar')) to stdout \\g | $pipe_cmd",
my $c4 = slurp_file($g_file);
like($c4, qr/foo.*bar/s);
+# Tests with pipelines. These trigger FATAL failures in the backend,
+# so they cannot be tested through the SQL regression tests.
+$node->safe_psql('postgres', 'CREATE TABLE psql_pipeline()');
+psql_fails_like(
+ $node,
+ qq{\\startpipeline
+COPY psql_pipeline FROM STDIN;
+SELECT 'val1';
+\\syncpipeline
+\\getresults
+\\endpipeline},
+ qr/server closed the connection unexpectedly/,
+ 'handling of protocol synchronization loss with pipelines');
+psql_fails_like(
+ $node,
+ qq{\\startpipeline
+COPY psql_pipeline FROM STDIN \\bind \\sendpipeline
+SELECT 'val1' \\bind \\sendpipeline
+\\syncpipeline
+\\getresults
+\\endpipeline},
+ qr/server closed the connection unexpectedly/,
+ 'handling of protocol synchronization loss with pipelines');
+
+# This time, test without the \getresults
+psql_fails_like(
+ $node,
+ qq{\\startpipeline
+COPY psql_pipeline FROM STDIN;
+SELECT 'val1';
+\\syncpipeline
+\\endpipeline},
+ qr/server closed the connection unexpectedly/,
+ 'handling of protocol synchronization loss with pipelines');
+psql_fails_like(
+ $node,
+ qq{\\startpipeline
+COPY psql_pipeline FROM STDIN \\bind \\sendpipeline
+SELECT 'val1' \\bind \\sendpipeline
+\\syncpipeline
+\\endpipeline},
+ qr/server closed the connection unexpectedly/,
+ 'handling of protocol synchronization loss with pipelines');
+
done_testing();
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v51 07/10] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index d6e446d582d..d4c1f0e7652 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -292,6 +292,18 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
+
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose anything.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
}
/*
@@ -523,21 +535,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could wait
- * indefinitely for our XID to complete. (The deadlock detector would
- * not recognize it because we'd be waiting for ourselves, i.e. no
- * real lock conflict.) It would be possible to run in a transaction
- * block if we had no XID, but this restriction is simpler for users
- * to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--r2slln3zpmilwu22
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v51-0008-Introduce-an-option-to-make-logical-replication-.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v53 5/7] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index f2759cdbef1..8d9b2b2e370 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -279,6 +279,21 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
/* Determine the lock mode to use. */
lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0);
+ if ((params.options & CLUOPT_CONCURRENT) != 0)
+ {
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose any functionality.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
+ }
+
/*
* If a single relation is specified, process it and we're done ... unless
* the relation is a partitioned table, in which case we fall through.
@@ -511,19 +526,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could deadlock
- * if there's an XID already assigned. It would be possible to run in
- * a transaction block if we had no XID, but this restriction is
- * simpler for users to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--qr3jlalmmcpkiodg
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v53-0006-Error-out-any-process-that-would-block-at-REPACK.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v53 5/7] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index f2759cdbef1..8d9b2b2e370 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -279,6 +279,21 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
/* Determine the lock mode to use. */
lockmode = RepackLockLevel((params.options & CLUOPT_CONCURRENT) != 0);
+ if ((params.options & CLUOPT_CONCURRENT) != 0)
+ {
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose any functionality.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
+ }
+
/*
* If a single relation is specified, process it and we're done ... unless
* the relation is a partitioned table, in which case we fall through.
@@ -511,19 +526,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could deadlock
- * if there's an XID already assigned. It would be possible to run in
- * a transaction block if we had no XID, but this restriction is
- * simpler for users to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--qr3jlalmmcpkiodg
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v53-0006-Error-out-any-process-that-would-block-at-REPACK.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v52 07/10] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index d6e446d582d..d4c1f0e7652 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -292,6 +292,18 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
+
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose anything.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
}
/*
@@ -523,21 +535,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could wait
- * indefinitely for our XID to complete. (The deadlock detector would
- * not recognize it because we'd be waiting for ourselves, i.e. no
- * real lock conflict.) It would be possible to run in a transaction
- * block if we had no XID, but this restriction is simpler for users
- * to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--gp2pyozrd5pweboh
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v52-0008-Introduce-an-option-to-make-logical-replication-.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v52 07/10] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index d6e446d582d..d4c1f0e7652 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -292,6 +292,18 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
+
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose anything.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
}
/*
@@ -523,21 +535,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could wait
- * indefinitely for our XID to complete. (The deadlock detector would
- * not recognize it because we'd be waiting for ourselves, i.e. no
- * real lock conflict.) It would be possible to run in a transaction
- * block if we had no XID, but this restriction is simpler for users
- * to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--gp2pyozrd5pweboh
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v52-0008-Introduce-an-option-to-make-logical-replication-.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
* [PATCH v51 07/10] Check for transaction block early in ExecRepack
@ 2026-04-03 15:23 Srinath Reddy Sadipiralla <[email protected]>
0 siblings, 0 replies; 50+ messages in thread
From: Srinath Reddy Sadipiralla @ 2026-04-03 15:23 UTC (permalink / raw)
Currently, executing REPACK (CONCURRENTLY) without a table name inside a
transaction block throws the error "REPACK CONCURRENTLY requires explicit
table name" instead of the expected transaction block error. This occurs
because ExecRepack() validates the parsed options and missing relation
before verifying the transaction state.
This behavior is inconsistent with other utility commands like VACUUM
,REINDEX, etc; which invoke PreventInTransactionBlock() at the very start
of their execution to properly reject execution inside user transactions
before validating targets.
Add PreventInTransactionBlock to the top of ExecRepack() to enforce the
transaction block restriction early. This prevents the user from fixing
a missing table error only to immediately hit a transaction block error,
and also ensures consistency with rest of the commands.
Author: Srinath Reddy Sadipiralla <[email protected]>
---
src/backend/commands/repack.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index d6e446d582d..d4c1f0e7652 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -292,6 +292,18 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool isTopLevel)
MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
+
+ /*
+ * Make sure we're not in a transaction block.
+ *
+ * The reason is that repack_setup_logical_decoding() could wait
+ * indefinitely for our XID to complete. (The deadlock detector would
+ * not recognize it because we'd be waiting for ourselves, i.e. no
+ * real lock conflict.) It would be possible to run in a transaction
+ * block if we had no XID, but this restriction is simpler for users
+ * to understand and we don't lose anything.
+ */
+ PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
}
/*
@@ -523,21 +535,7 @@ cluster_rel(RepackCommand cmd, Relation OldHeap, Oid indexOid,
* replica index OID.
*/
if (concurrent)
- {
- /*
- * Make sure we're not in a transaction block.
- *
- * The reason is that repack_setup_logical_decoding() could wait
- * indefinitely for our XID to complete. (The deadlock detector would
- * not recognize it because we'd be waiting for ourselves, i.e. no
- * real lock conflict.) It would be possible to run in a transaction
- * block if we had no XID, but this restriction is simpler for users
- * to understand and we don't lose anything.
- */
- PreventInTransactionBlock(isTopLevel, "REPACK (CONCURRENTLY)");
-
check_repack_concurrently_requirements(OldHeap, &ident_idx);
- }
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
--
2.47.3
--r2slln3zpmilwu22
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v51-0008-Introduce-an-option-to-make-logical-replication-.patch"
^ permalink raw reply [nested|flat] 50+ messages in thread
end of thread, other threads:[~2026-04-03 15:23 UTC | newest]
Thread overview: 50+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-03-05 00:20 Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment Lukas Fittl <[email protected]>
2023-03-07 09:51 ` David Rowley <[email protected]>
2023-07-06 07:56 ` Daniel Gustafsson <[email protected]>
2023-07-06 08:27 ` Lukas Fittl <[email protected]>
2025-03-20 08:47 ` Ilia Evdokimov <[email protected]>
2025-03-20 10:37 ` David Rowley <[email protected]>
2025-03-20 12:04 ` Ilia Evdokimov <[email protected]>
2025-03-20 12:32 ` Andrei Lepikhov <[email protected]>
2025-03-20 14:03 ` Ilia Evdokimov <[email protected]>
2025-03-20 18:54 ` Andrei Lepikhov <[email protected]>
2025-03-21 02:50 ` David Rowley <[email protected]>
2025-03-21 09:02 ` Andrei Lepikhov <[email protected]>
2025-03-21 10:08 ` Ilia Evdokimov <[email protected]>
2025-03-23 21:16 ` David Rowley <[email protected]>
2025-03-24 21:23 ` Andrei Lepikhov <[email protected]>
2025-03-24 22:05 ` David Rowley <[email protected]>
2025-03-24 22:15 ` Tom Lane <[email protected]>
2025-03-24 22:30 ` Lukas Fittl <[email protected]>
2025-03-24 22:45 ` Tom Lane <[email protected]>
2025-03-25 09:32 ` Andrei Lepikhov <[email protected]>
2025-03-26 19:37 ` Robert Haas <[email protected]>
2025-03-27 10:12 ` Ilia Evdokimov <[email protected]>
2025-03-27 12:46 ` Robert Haas <[email protected]>
2025-03-27 13:46 ` Andrei Lepikhov <[email protected]>
2025-03-28 12:20 ` Ilia Evdokimov <[email protected]>
2025-04-01 07:52 ` Ilia Evdokimov <[email protected]>
2025-03-25 03:32 ` David Rowley <[email protected]>
2025-03-25 06:40 ` Andrei Lepikhov <[email protected]>
2025-03-07 00:05 Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-07 08:08 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-07 22:31 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-17 09:50 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-17 10:19 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 00:50 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 08:55 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2025-03-18 09:27 ` Re: Add Pipelining support in psql Jelte Fennema-Nio <[email protected]>
2025-03-19 02:28 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-19 13:05 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-19 04:49 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-03-18 09:36 ` Re: Add Pipelining support in psql Daniel Verite <[email protected]>
2025-03-18 23:56 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-04-16 14:31 Re: Add Pipelining support in psql a.kozhemyakin <[email protected]>
2025-04-16 16:18 ` Re: Add Pipelining support in psql Michael Paquier <[email protected]>
2025-04-22 12:37 ` Re: Add Pipelining support in psql Anthonin Bonnefoy <[email protected]>
2026-04-03 15:23 [PATCH v51 07/10] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
2026-04-03 15:23 [PATCH v53 5/7] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
2026-04-03 15:23 [PATCH v53 5/7] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
2026-04-03 15:23 [PATCH v52 07/10] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
2026-04-03 15:23 [PATCH v52 07/10] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
2026-04-03 15:23 [PATCH v51 07/10] Check for transaction block early in ExecRepack Srinath Reddy Sadipiralla <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox