On 29.06.18 05:15, Jeff Janes wrote:
> Since pg_dump calls pg_get_expr once over and over again on the same
> table consecutively, perhaps we could cache the column alias assignments
> in a single-entry cache, so if it is called on the same table as last
> time it just re-uses the aliases from last time. I am not planning on
> working on that, I don't know where such a cache could be stored such
> that is freed and invalidated at the appropriate times.
I looked into that. deparse_context_for() is actually not that
expensive on its own, well below one second, but it gets somewhat
expensive when you call it 1600 times for one table. So to address that
case, we can cache the deparse context between calls in the fn_extra
field of pg_get_expr. The attached patch does that. This makes the
pg_dump -s times pretty much constant even with 1600 columns with
defaults. psql \d should benefit similarly. I haven't seen any other
cases where we'd expect hundreds of related objects to deparse. (Do
people have hundreds of policies per table?)
(I suppose you could create scenarios with very many such tables to make
the overhead visible again.)
How realistic is this use case? Is it worth it?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 15621a2c8bbff2f72c2df8f7afd24910a51289c9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 5 Jul 2018 16:27:44 +0200
Subject: [PATCH] Cache pg_get_expr deparse context between calls
---
src/backend/utils/adt/ruleutils.c | 49 ++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 065238b0fe..aa73bb35d6 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -327,7 +327,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int
prettyFlags,
bool attrsOnly, bool
missing_ok);
static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
int prettyFlags, bool
missing_ok);
-static text *pg_get_expr_worker(text *expr, Oid relid, const char *relname,
+static text *pg_get_expr_worker(FunctionCallInfo fcinfo, text *expr, Oid
relid, const char *relname,
int prettyFlags);
static int print_function_arguments(StringInfo buf, HeapTuple proctup,
bool print_table_args, bool
print_defaults);
@@ -2292,7 +2292,7 @@ pg_get_expr(PG_FUNCTION_ARGS)
else
relname = NULL;
- PG_RETURN_TEXT_P(pg_get_expr_worker(expr, relid, relname, prettyFlags));
+ PG_RETURN_TEXT_P(pg_get_expr_worker(fcinfo, expr, relid, relname,
prettyFlags));
}
Datum
@@ -2317,14 +2317,20 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
else
relname = NULL;
- PG_RETURN_TEXT_P(pg_get_expr_worker(expr, relid, relname, prettyFlags));
+ PG_RETURN_TEXT_P(pg_get_expr_worker(fcinfo, expr, relid, relname,
prettyFlags));
}
+struct cached_context
+{
+ Oid relid;
+ List *context;
+};
+
static text *
-pg_get_expr_worker(text *expr, Oid relid, const char *relname, int prettyFlags)
+pg_get_expr_worker(FunctionCallInfo fcinfo, text *expr, Oid relid, const char
*relname, int prettyFlags)
{
Node *node;
- List *context;
+ List *context = NIL;
char *exprstr;
char *str;
@@ -2338,9 +2344,36 @@ pg_get_expr_worker(text *expr, Oid relid, const char
*relname, int prettyFlags)
/* Prepare deparse context if needed */
if (OidIsValid(relid))
- context = deparse_context_for(relname, relid);
- else
- context = NIL;
+ {
+ struct cached_context *cc;
+
+ if (fcinfo->flinfo && fcinfo->flinfo->fn_extra)
+ {
+ cc = fcinfo->flinfo->fn_extra;
+ if (cc->relid == relid)
+ context = cc->context;
+ }
+
+ if (!context)
+ {
+ MemoryContext oldcontext = NULL;
+ struct cached_context *cc;
+
+ if (fcinfo->flinfo)
+ oldcontext =
MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt);
+
+ context = deparse_context_for(relname, relid);
+
+ if (fcinfo->flinfo)
+ {
+ cc = palloc(sizeof(*cc));
+ cc->relid = relid;
+ cc->context = context;
+ fcinfo->flinfo->fn_extra = cc;
+ MemoryContextSwitchTo(oldcontext);
+ }
+ }
+ }
/* Deparse */
str = deparse_expression_pretty(node, context, false, false,
base-commit: f61988d160c1af8c1ed495e5c547726e88a45036
--
2.18.0
