Deparsing rewritten query
Hi,
I sometimes have to deal with queries referencing multiple and/or complex
views. In such cases, it's quite troublesome to figure out what is the query
really executed. Debug_print_rewritten isn't really useful for non trivial
queries, and manually doing the view expansion isn't great either.
While not being ideal, I wouldn't mind using a custom extension for that but
this isn't an option as get_query_def() is private and isn't likely to change.
As an alternative, maybe we could expose a simple SRF that would take care of
rewriting the query and deparsing the resulting query tree(s)?
I'm attaching a POC patch for that, adding a new pg_get_query_def(text) SRF.
Usage example:
SELECT pg_get_query_def('SELECT * FROM shoe') as def;
def
--------------------------------------------------------
SELECT shoename, +
sh_avail, +
slcolor, +
slminlen, +
slminlen_cm, +
slmaxlen, +
slmaxlen_cm, +
slunit +
FROM ( SELECT sh.shoename, +
sh.sh_avail, +
sh.slcolor, +
sh.slminlen, +
(sh.slminlen * un.un_fact) AS slminlen_cm,+
sh.slmaxlen, +
(sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
sh.slunit +
FROM shoe_data sh, +
unit un +
WHERE (sh.slunit = un.un_name)) shoe; +
(1 row)
Attachments:
v1-0001-Add-pg_get_query_def-to-deparse-and-print-a-rewri.patchtext/x-diff; charset=us-asciiDownload
From 5e726861fff28a276bb2e31972b278b833968ff4 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Sun, 27 Jun 2021 11:39:47 +0800
Subject: [PATCH v1] Add pg_get_query_def() to deparse and print a rewritten
SQL statement.
---
src/backend/rewrite/rewriteHandler.c | 2 +
src/backend/utils/adt/ruleutils.c | 70 ++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 3 ++
src/test/regress/expected/rules.out | 26 +++++++++++
src/test/regress/sql/rules.sql | 3 ++
5 files changed, 104 insertions(+)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 88a9e95e33..f01b4531bf 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1815,6 +1815,8 @@ ApplyRetrieveRule(Query *parsetree,
rte->rtekind = RTE_SUBQUERY;
rte->subquery = rule_action;
rte->security_barrier = RelationIsSecurityView(relation);
+ if (!rte->alias)
+ rte->alias = makeAlias(get_rel_name(rte->relid), NULL);
/* Clear fields that should not be set in a subquery RTE */
rte->relid = InvalidOid;
rte->relkind = 0;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3719755a0d..cd39f4ce75 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -49,6 +49,8 @@
#include "nodes/nodeFuncs.h"
#include "nodes/pathnodes.h"
#include "optimizer/optimizer.h"
+#include "parser/analyze.h"
+#include "parser/parse_node.h"
#include "parser/parse_agg.h"
#include "parser/parse_func.h"
#include "parser/parse_node.h"
@@ -58,6 +60,7 @@
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
#include "rewrite/rewriteSupport.h"
+#include "tcop/tcopprot.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -489,6 +492,73 @@ static void get_reloptions(StringInfo buf, Datum reloptions);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
+/* return the query as postgres will rewrite */
+Datum
+pg_get_query_def(PG_FUNCTION_ARGS)
+{
+ char *sql = TextDatumGetCString(PG_GETARG_TEXT_PP(0));
+ List *parsetree_list;
+ List *querytree_list;
+ RawStmt *parsetree;
+ Query *query;
+ bool snapshot_set = false;
+ StringInfoData buf;
+ StringInfoData res;
+ ListCell *lc;
+
+ if (strcmp(sql, "") == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("empty statement now allowed")));
+
+ parsetree_list = pg_parse_query(sql);
+
+ /* only support one statement at a time */
+ if (list_length(parsetree_list) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("a single statement should be provided")));
+
+ initStringInfo(&res);
+
+ parsetree = linitial_node(RawStmt, parsetree_list);
+
+ /*
+ * Set up a snapshot if parse analysis/planning will need one.
+ */
+ if (analyze_requires_snapshot(parsetree))
+ {
+ PushActiveSnapshot(GetTransactionSnapshot());
+ snapshot_set = true;
+ }
+
+ querytree_list = pg_analyze_and_rewrite(parsetree, sql,
+ NULL, 0, NULL);
+
+ /* Done with the snapshot used for parsing/planning */
+ if (snapshot_set)
+ PopActiveSnapshot();
+
+ foreach(lc, querytree_list)
+ {
+ query = (Query *) lfirst(lc);
+ initStringInfo(&buf);
+
+ if (query->utilityStmt)
+ appendStringInfo(&res, "%s;\n", sql);
+ else
+ {
+ get_query_def(query, &buf, NIL, NULL,
+ PRETTYFLAG_INDENT,
+ WRAP_COLUMN_DEFAULT, 0);
+
+ appendStringInfo(&res, "%s;\n", buf.data);
+ }
+ }
+ pfree(buf.data);
+
+ PG_RETURN_TEXT_P(string_to_text(res.data));
+}
/* ----------
* pg_get_ruledef - Do it all and return a text
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fde251fa4f..b15b40aa7a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3650,6 +3650,9 @@
proargtypes => 'oid oid', prosrc => 'oidge' },
# System-view support functions
+{ oid => '9246', descr => 'show a query as rewritten',
+ proname => 'pg_get_query_def', provolatile => 'v', prorettype => 'text',
+ proargtypes => 'text', prosrc => 'pg_get_query_def' },
{ oid => '1573', descr => 'source text of a rule',
proname => 'pg_get_ruledef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_ruledef' },
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e5ab11275d..dd88b890f4 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3075,6 +3075,32 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
WHERE sh.slunit = un.un_name;
(1 row)
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+ def
+--------------------------------------------------------
+ SELECT shoename, +
+ sh_avail, +
+ slcolor, +
+ slminlen, +
+ slminlen_cm, +
+ slmaxlen, +
+ slmaxlen_cm, +
+ slunit +
+ FROM ( SELECT sh.shoename, +
+ sh.sh_avail, +
+ sh.slcolor, +
+ sh.slminlen, +
+ (sh.slminlen * un.un_fact) AS slminlen_cm,+
+ sh.slmaxlen, +
+ (sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
+ sh.slunit +
+ FROM shoe_data sh, +
+ unit un +
+ WHERE (sh.slunit = un.un_name)) shoe; +
+
+(1 row)
+
--
-- check multi-row VALUES in rules
--
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 6ec37c4381..aeac713680 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -998,6 +998,9 @@ select pg_get_viewdef('shoe'::regclass) as unpretty;
select pg_get_viewdef('shoe'::regclass,true) as pretty;
select pg_get_viewdef('shoe'::regclass,0) as prettier;
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+
--
-- check multi-row VALUES in rules
--
--
2.31.1
ne 27. 6. 2021 v 6:11 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
Hi,
I sometimes have to deal with queries referencing multiple and/or complex
views. In such cases, it's quite troublesome to figure out what is the
query
really executed. Debug_print_rewritten isn't really useful for non trivial
queries, and manually doing the view expansion isn't great either.While not being ideal, I wouldn't mind using a custom extension for that
but
this isn't an option as get_query_def() is private and isn't likely to
change.As an alternative, maybe we could expose a simple SRF that would take care
of
rewriting the query and deparsing the resulting query tree(s)?I'm attaching a POC patch for that, adding a new pg_get_query_def(text)
SRF.Usage example:
SELECT pg_get_query_def('SELECT * FROM shoe') as def;
def
--------------------------------------------------------
SELECT shoename, +
sh_avail, +
slcolor, +
slminlen, +
slminlen_cm, +
slmaxlen, +
slmaxlen_cm, +
slunit +
FROM ( SELECT sh.shoename, +
sh.sh_avail, +
sh.slcolor, +
sh.slminlen, +
(sh.slminlen * un.un_fact) AS slminlen_cm,+
sh.slmaxlen, +
(sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
sh.slunit +
FROM shoe_data sh, +
unit un +
WHERE (sh.slunit = un.un_name)) shoe; +(1 row)
+1
Pavel
Hi,
I sometimes have to deal with queries referencing multiple and/or complex
views. In such cases, it's quite troublesome to figure out what is the
query
really executed. Debug_print_rewritten isn't really useful for non trivial
queries, and manually doing the view expansion isn't great either.While not being ideal, I wouldn't mind using a custom extension for that
but
this isn't an option as get_query_def() is private and isn't likely to
change.As an alternative, maybe we could expose a simple SRF that would take care
of
rewriting the query and deparsing the resulting query tree(s)?I'm attaching a POC patch for that, adding a new pg_get_query_def(text)
SRF.
+1
If you don't mind, I made small corrections to your patch.
1. strcmp(sql, "") could not be replaced by sql[0] == '\0'?
2. the error message would not be: "empty statement not allowed"?
3. initStringInfo(&buf) inside a loop, wouldn't it be exaggerated? each
time call palloc0.
regards,
Ranier Vilela
Attachments:
v1-0002-Add-pg_get_query_def-to-deparse-and-print-a-rewri.patchapplication/octet-stream; name=v1-0002-Add-pg_get_query_def-to-deparse-and-print-a-rewri.patchDownload
From 5e726861fff28a276bb2e31972b278b833968ff4 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Sun, 27 Jun 2021 11:39:47 +0800
Subject: [PATCH v1] Add pg_get_query_def() to deparse and print a rewritten
SQL statement.
---
src/backend/rewrite/rewriteHandler.c | 2 +
src/backend/utils/adt/ruleutils.c | 70 ++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 3 ++
src/test/regress/expected/rules.out | 26 +++++++++++
src/test/regress/sql/rules.sql | 3 ++
5 files changed, 104 insertions(+)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 88a9e95e33..f01b4531bf 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1815,6 +1815,8 @@ ApplyRetrieveRule(Query *parsetree,
rte->rtekind = RTE_SUBQUERY;
rte->subquery = rule_action;
rte->security_barrier = RelationIsSecurityView(relation);
+ if (!rte->alias)
+ rte->alias = makeAlias(get_rel_name(rte->relid), NULL);
/* Clear fields that should not be set in a subquery RTE */
rte->relid = InvalidOid;
rte->relkind = 0;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3719755a0d..cd39f4ce75 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -49,6 +49,8 @@
#include "nodes/nodeFuncs.h"
#include "nodes/pathnodes.h"
#include "optimizer/optimizer.h"
+#include "parser/analyze.h"
+#include "parser/parse_node.h"
#include "parser/parse_agg.h"
#include "parser/parse_func.h"
#include "parser/parse_node.h"
@@ -58,6 +60,7 @@
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
#include "rewrite/rewriteSupport.h"
+#include "tcop/tcopprot.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -489,6 +492,73 @@ static void get_reloptions(StringInfo buf, Datum reloptions);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
+/* return the query that postgres will rewrite */
+Datum
+pg_get_query_def(PG_FUNCTION_ARGS)
+{
+ char *sql = TextDatumGetCString(PG_GETARG_TEXT_PP(0));
+ List *parsetree_list;
+ List *querytree_list;
+ RawStmt *parsetree;
+ bool snapshot_set = false;
+ StringInfoData buf;
+ StringInfoData res;
+ ListCell *lc;
+
+ if (sql[0] == '\0')
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("empty statement not allowed")));
+
+ parsetree_list = pg_parse_query(sql);
+
+ /* only support one statement at a time */
+ if (list_length(parsetree_list) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("a single statement should be provided")));
+
+ parsetree = linitial_node(RawStmt, parsetree_list);
+
+ /*
+ * Set up a snapshot if parse analysis/planning will need one.
+ */
+ if (analyze_requires_snapshot(parsetree))
+ {
+ PushActiveSnapshot(GetTransactionSnapshot());
+ snapshot_set = true;
+ }
+
+ querytree_list = pg_analyze_and_rewrite(parsetree, sql,
+ NULL, 0, NULL);
+
+ /* Done with the snapshot used for parsing/planning */
+ if (snapshot_set)
+ PopActiveSnapshot();
+
+ initStringInfo(&res);
+ initStringInfo(&buf);
+ foreach(lc, querytree_list)
+ {
+ Query *query;
+
+ query = (Query *) lfirst(lc);
+
+ if (query->utilityStmt)
+ appendStringInfo(&res, "%s;\n", sql);
+ else
+ {
+ resetStringInfo(&buf);
+ get_query_def(query, &buf, NIL, NULL,
+ PRETTYFLAG_INDENT,
+ WRAP_COLUMN_DEFAULT, 0);
+ appendStringInfo(&res, "%s;\n", buf.data);
+ }
+ }
+ pfree(buf.data);
+
+ PG_RETURN_TEXT_P(string_to_text(res.data));
+}
/* ----------
* pg_get_ruledef - Do it all and return a text
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fde251fa4f..b15b40aa7a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3650,6 +3650,9 @@
proargtypes => 'oid oid', prosrc => 'oidge' },
# System-view support functions
+{ oid => '9246', descr => 'show a query as rewritten',
+ proname => 'pg_get_query_def', provolatile => 'v', prorettype => 'text',
+ proargtypes => 'text', prosrc => 'pg_get_query_def' },
{ oid => '1573', descr => 'source text of a rule',
proname => 'pg_get_ruledef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_ruledef' },
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e5ab11275d..dd88b890f4 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3075,6 +3075,32 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
WHERE sh.slunit = un.un_name;
(1 row)
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+ def
+--------------------------------------------------------
+ SELECT shoename, +
+ sh_avail, +
+ slcolor, +
+ slminlen, +
+ slminlen_cm, +
+ slmaxlen, +
+ slmaxlen_cm, +
+ slunit +
+ FROM ( SELECT sh.shoename, +
+ sh.sh_avail, +
+ sh.slcolor, +
+ sh.slminlen, +
+ (sh.slminlen * un.un_fact) AS slminlen_cm,+
+ sh.slmaxlen, +
+ (sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
+ sh.slunit +
+ FROM shoe_data sh, +
+ unit un +
+ WHERE (sh.slunit = un.un_name)) shoe; +
+
+(1 row)
+
--
-- check multi-row VALUES in rules
--
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 6ec37c4381..aeac713680 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -998,6 +998,9 @@ select pg_get_viewdef('shoe'::regclass) as unpretty;
select pg_get_viewdef('shoe'::regclass,true) as pretty;
select pg_get_viewdef('shoe'::regclass,0) as prettier;
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+
--
-- check multi-row VALUES in rules
--
--
2.31.1
Import Notes
Resolved by subject fallback
On Sun, Jun 27, 2021 at 10:06:00AM -0300, Ranier Vilela wrote:
1. strcmp(sql, "") could not be replaced by sql[0] == '\0'?
It's a debugging leftover that I forgot to remove. There's no point trying
to catch an empty string as e.g. a single space would be exactly the same, and
both would be caught by the next (and correct) test.
3. initStringInfo(&buf) inside a loop, wouldn't it be exaggerated? each
time call palloc0.
initStringInfo calls palloc, not palloc0.
It's unlikely to make any difference. Rules have been strongly discouraged for
many years, and if you have enough to make a noticeable difference here, you
probably have bigger problems. But no objection to reset the StringInfo
instead.
Julien Rouhaud <rjuju123@gmail.com> writes:
As an alternative, maybe we could expose a simple SRF that would take care of
rewriting the query and deparsing the resulting query tree(s)?
I'm not really excited by this, as it seems like it's exposing internal
decisions we could change someday; to wit, first that there is any such
thing as a separate rewriting pass, and second that its output is
interpretable as pure SQL. (TBH, I'm not 100% sure that the second
assumption is true even today, although I know there are ancient comments
that claim that.) It's not very hard to imagine someday moving view
expansion into the planner on efficiency grounds, leaving the rewriter
handling only the rare uses of INSERT/UPDATE/DELETE rules.
If there's functions in ruleutils.c that we'd need to make public to
let somebody write a debugging extension that does this kind of thing,
I'd be happier with that approach than with creating a core-server SQL
function for it. There might be more than one use-case for the
exposed bits.
regards, tom lane
On Sun, Jun 27, 2021 at 10:34:52AM -0400, Tom Lane wrote:
I'm not really excited by this, as it seems like it's exposing internal
decisions we could change someday; to wit, first that there is any such
thing as a separate rewriting pass
Sure, but the fact that views will significantly impact the query being
executed from the one written is not an internal decision. In my opinion
knowing what the final "real" query will be is still a valid concern, whether
we have a rewriting pass or not.
and second that its output is
interpretable as pure SQL. (TBH, I'm not 100% sure that the second
assumption is true even today, although I know there are ancient comments
that claim that.)
I totally agree. Note that there was at least one gotcha handled in this
patch: rewritten views didn't get an alias, which is mandatory for an SQL
query.
It's not very hard to imagine someday moving view
expansion into the planner on efficiency grounds, leaving the rewriter
handling only the rare uses of INSERT/UPDATE/DELETE rules.
Agreed. One the other hand having such a function in core may ensure that any
significant change in those area will keep an API to retrieve the final query
representation.
If there's functions in ruleutils.c that we'd need to make public to
let somebody write a debugging extension that does this kind of thing,
I'd be happier with that approach than with creating a core-server SQL
function for it. There might be more than one use-case for the
exposed bits.
It would mean exposing at least get_query_def(). I thought that exposing this
function was already suggested and refused, but I may be wrong. Maybe other
people would like to have nearby functions exposed too.
Note that if we go this way, we would still need at least something like this
patch's chunk in rewriteHandler.c applied, as otherwise the vast majority of
rewritten and deparsed queries won't be valid.
Julien Rouhaud <rjuju123@gmail.com> writes:
On Sun, Jun 27, 2021 at 10:34:52AM -0400, Tom Lane wrote:
It's not very hard to imagine someday moving view
expansion into the planner on efficiency grounds, leaving the rewriter
handling only the rare uses of INSERT/UPDATE/DELETE rules.
Agreed. One the other hand having such a function in core may ensure that any
significant change in those area will keep an API to retrieve the final query
representation.
My point is precisely that I'm unwilling to make such a promise.
I do not buy that this capability is worth very much, given that
we've gotten along fine without it for twenty-plus years. If you
want to have it as an internal, might-change-at-any-time API,
that seems all right. If you're trying to lock it down as something
that will be there forevermore, you're likely to end up with nothing.
regards, tom lane
On Sun, Jun 27, 2021 at 11:14:05AM -0400, Tom Lane wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
Agreed. One the other hand having such a function in core may ensure that any
significant change in those area will keep an API to retrieve the final query
representation.My point is precisely that I'm unwilling to make such a promise.
I do not buy that this capability is worth very much, given that
we've gotten along fine without it for twenty-plus years. If you
want to have it as an internal, might-change-at-any-time API,
that seems all right.
I'm totally fine with a might-change-at-any-time API, but not with a
might-disappear-at-anytime API. If exposing get_query_def() can become
virtually useless in a few releases with no hope to get required in-core change
for retrieving the final query representation, I agree this we can stop the
discussion here.
Le 27/06/2021 à 17:14, Tom Lane a écrit :
Julien Rouhaud <rjuju123@gmail.com> writes:
On Sun, Jun 27, 2021 at 10:34:52AM -0400, Tom Lane wrote:
It's not very hard to imagine someday moving view
expansion into the planner on efficiency grounds, leaving the rewriter
handling only the rare uses of INSERT/UPDATE/DELETE rules.Agreed. One the other hand having such a function in core may ensure that any
significant change in those area will keep an API to retrieve the final query
representation.My point is precisely that I'm unwilling to make such a promise.
I do not buy that this capability is worth very much, given that
we've gotten along fine without it for twenty-plus years. If you
want to have it as an internal, might-change-at-any-time API,
that seems all right. If you're trying to lock it down as something
that will be there forevermore, you're likely to end up with nothing.regards, tom lane
I have to say that such feature would be very helpful for some DBA and
especially migration work. The problem is when you have tons of views
that call other views in the from or join clauses. These views also call
other views, etc. I have had instances where there were up to 25 nested
views calls. When you want to clean up this kind of code, using
get_query_def () will help save a lot of manual rewrite time and
headache to get the final code executed.
If we could at least call get_query_def()through an extension if we
didn't have a functionit would be ideal for DBAs.I agree this is unusual
but when it does happen to you being able to call get_query_def () helps
a lot.
--
Gilles Darold
http://www.darold.net/
Thanks for the feedback Gilles!
On Mon, Jun 28, 2021 at 04:06:54PM +0200, Gilles Darold wrote:
If we could at least call get_query_def()through an extension if we didn't
have a functionit would be ideal for DBAs.I agree this is unusual but when
it does happen to you being able to call get_query_def () helps a lot.
Since at least 2 other persons seems to be interested in that feature, I can
take care of writing and maintaining such an extension, provided that the
required infrastructure is available in core.
PFA v2 of the patch which only adds the required alias and expose
get_query_def().
Attachments:
v2-0001-Expose-get_query_def.patchtext/x-diff; charset=us-asciiDownload
From d93ea00f3a63f0799538b08208439c01bf7a8aba Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Tue, 29 Jun 2021 00:07:04 +0800
Subject: [PATCH v2] Expose get_query_def()
This function can be useful for external module, for instance if they want to
display a statement after the rewrite stage.
In order to emit valid SQL, ApplyRetrieveRule now also adds an alias using the
original rule name if no alias were present.
Author: Julien Rouhaud
Discussion: https://postgr.es/m/20210627041138.zklczwmu3ms4ufnk@nol
---
src/backend/rewrite/rewriteHandler.c | 2 ++
src/backend/utils/adt/ruleutils.c | 5 +----
src/include/utils/ruleutils.h | 3 +++
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 88a9e95e33..f01b4531bf 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1815,6 +1815,8 @@ ApplyRetrieveRule(Query *parsetree,
rte->rtekind = RTE_SUBQUERY;
rte->subquery = rule_action;
rte->security_barrier = RelationIsSecurityView(relation);
+ if (!rte->alias)
+ rte->alias = makeAlias(get_rel_name(rte->relid), NULL);
/* Clear fields that should not be set in a subquery RTE */
rte->relid = InvalidOid;
rte->relkind = 0;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3719755a0d..2844239daa 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -387,9 +387,6 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags, int wrapColumn);
-static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
- TupleDesc resultDesc,
- int prettyFlags, int wrapColumn, int startIndent);
static void get_values_def(List *values_lists, deparse_context *context);
static void get_with_clause(Query *query, deparse_context *context);
static void get_select_query_def(Query *query, deparse_context *context,
@@ -5251,7 +5248,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
* the view represented by a SELECT query.
* ----------
*/
-static void
+void
get_query_def(Query *query, StringInfo buf, List *parentnamespace,
TupleDesc resultDesc,
int prettyFlags, int wrapColumn, int startIndent)
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index d333e5e8a5..384cbc101a 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -39,6 +39,9 @@ extern List *select_rtable_names_for_explain(List *rtable,
Bitmapset *rels_used);
extern char *generate_collation_name(Oid collid);
extern char *generate_opclass_name(Oid opclass);
+void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
+ TupleDesc resultDesc,
+ int prettyFlags, int wrapColumn, int startIndent);
extern char *get_range_partbound_string(List *bound_datums);
extern char *pg_get_statisticsobjdef_string(Oid statextid);
--
2.31.1
Le 28/06/2021 à 18:41, Julien Rouhaud a écrit :
Thanks for the feedback Gilles!
On Mon, Jun 28, 2021 at 04:06:54PM +0200, Gilles Darold wrote:
If we could at least call get_query_def()through an extension if we didn't
have a functionit would be ideal for DBAs.I agree this is unusual but when
it does happen to you being able to call get_query_def () helps a lot.Since at least 2 other persons seems to be interested in that feature, I can
take care of writing and maintaining such an extension, provided that the
required infrastructure is available in core.PFA v2 of the patch which only adds the required alias and expose
get_query_def().
Thanks a lot Julien, such facilities are really helpful for DBAs and
make the difference with other RDBMS. I don't think that this feature
exists else where.
--
Gilles Darold
http://www.darold.net/
Hi
po 31. 1. 2022 v 15:37 odesílatel Gilles Darold <gilles@darold.net> napsal:
Le 28/06/2021 à 18:41, Julien Rouhaud a écrit :
Thanks for the feedback Gilles!
On Mon, Jun 28, 2021 at 04:06:54PM +0200, Gilles Darold wrote:
If we could at least call get_query_def()through an extension if we
didn't
have a functionit would be ideal for DBAs.I agree this is unusual but
when
it does happen to you being able to call get_query_def () helps a lot.
Since at least 2 other persons seems to be interested in that feature, I
can
take care of writing and maintaining such an extension, provided that the
required infrastructure is available in core.PFA v2 of the patch which only adds the required alias and expose
get_query_def().
I checked the last patch. I think it is almost trivial. I miss just
comment, why this alias is necessary
+ if (!rte->alias)
+ rte->alias = makeAlias(get_rel_name(rte->relid), NULL);Regards
Pavel
Show quoted text
Thanks a lot Julien, such facilities are really helpful for DBAs and
make the difference with other RDBMS. I don't think that this feature
exists else where.--
Gilles Darold
http://www.darold.net/
Hi,
On Mon, Jan 31, 2022 at 06:46:37PM +0100, Pavel Stehule wrote:
I checked the last patch. I think it is almost trivial. I miss just
comment, why this alias is necessary+ if (!rte->alias) + rte->alias = makeAlias(get_rel_name(rte->relid), NULL);
Thanks for looking at it Pavel!
The alias is necessary because otherwise queries involving views won't produce
valid SQL, as aliases for subquery is mandatory. This was part of the v1
regression tests:
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+ def
+--------------------------------------------------------
+ SELECT shoename, +
+ sh_avail, +
+ slcolor, +
+ slminlen, +
+ slminlen_cm, +
+ slmaxlen, +
+ slmaxlen_cm, +
+ slunit +
+ FROM ( SELECT sh.shoename, +
+ sh.sh_avail, +
+ sh.slcolor, +
+ sh.slminlen, +
+ (sh.slminlen * un.un_fact) AS slminlen_cm,+
+ sh.slmaxlen, +
+ (sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
+ sh.slunit +
+ FROM shoe_data sh, +
+ unit un +
+ WHERE (sh.slunit = un.un_name)) shoe; +the mandatory "shoe" alias is added with that change.
I looked for other similar problems and didn't find anything, but given the
complexity of the SQL standard it's quite possible that I missed some other
corner case.
po 31. 1. 2022 v 19:09 odesílatel Julien Rouhaud <rjuju123@gmail.com>
napsal:
Hi,
On Mon, Jan 31, 2022 at 06:46:37PM +0100, Pavel Stehule wrote:
I checked the last patch. I think it is almost trivial. I miss just
comment, why this alias is necessary+ if (!rte->alias) + rte->alias = makeAlias(get_rel_name(rte->relid), NULL);Thanks for looking at it Pavel!
The alias is necessary because otherwise queries involving views won't
produce
valid SQL, as aliases for subquery is mandatory. This was part of the v1
regression tests:+-- test pg_get_query_def() +SELECT pg_get_query_def('SELECT * FROM shoe') as def; + def +-------------------------------------------------------- + SELECT shoename, + + sh_avail, + + slcolor, + + slminlen, + + slminlen_cm, + + slmaxlen, + + slmaxlen_cm, + + slunit + + FROM ( SELECT sh.shoename, + + sh.sh_avail, + + sh.slcolor, + + sh.slminlen, + + (sh.slminlen * un.un_fact) AS slminlen_cm,+ + sh.slmaxlen, + + (sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+ + sh.slunit + + FROM shoe_data sh, + + unit un + + WHERE (sh.slunit = un.un_name)) shoe; +the mandatory "shoe" alias is added with that change.
I looked for other similar problems and didn't find anything, but given the
complexity of the SQL standard it's quite possible that I missed some other
corner case.
I don't feel good about forcing an alias. relname doesn't ensure
uniqueness. You can have two views with the same name from different
schemas. Moreover this field is necessary only when a deparsed query is
printed, not always.
Isn't possible to compute the correct subquery alias in print time when it
is missing?
Regards
Pavel
Hi,
On Mon, Jan 31, 2022 at 10:05:44PM +0100, Pavel Stehule wrote:
I don't feel good about forcing an alias. relname doesn't ensure
uniqueness. You can have two views with the same name from different
schemas. Moreover this field is necessary only when a deparsed query is
printed, not always.
Yes I agree.
Isn't possible to compute the correct subquery alias in print time when it
is missing?
Actually I think that the current code already does everything to generate
unique refnames, it's just that they don't get printed for a query after view
expansions. I modified the patch to simply make sure that an alias is
displayed when it's a subquery and the output using a custom pg_get_query_def
is like that:
# select pg_get_query_def('select * from nsp1.v1');
pg_get_query_def
-------------------------------
SELECT nb +
FROM ( SELECT 1 AS nb) v1;+
(1 row)
# select pg_get_query_def('select * from nsp1.v1, nsp2.v1');
pg_get_query_def
-------------------------------
SELECT v1.nb, +
v1_1.nb +
FROM ( SELECT 1 AS nb) v1,+
( SELECT 1 AS nb) v1_1; +
(1 row)
Attachments:
v3-0001-Add-pg_get_query_def-to-deparse-and-print-a-rewri.patchtext/plain; charset=us-asciiDownload
From 093cc350c09b3e0a458822da7f541ca602af4ef6 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Sun, 27 Jun 2021 11:39:47 +0800
Subject: [PATCH v3] Add pg_get_query_def() to deparse and print a rewritten
SQL statement.
---
src/backend/utils/adt/ruleutils.c | 75 +++++++++++++++++++++++++++++
src/include/catalog/pg_proc.dat | 3 ++
src/test/regress/expected/rules.out | 26 ++++++++++
src/test/regress/sql/rules.sql | 3 ++
4 files changed, 107 insertions(+)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 039b1d2b95..1186438757 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -49,6 +49,8 @@
#include "nodes/nodeFuncs.h"
#include "nodes/pathnodes.h"
#include "optimizer/optimizer.h"
+#include "parser/analyze.h"
+#include "parser/parse_node.h"
#include "parser/parse_agg.h"
#include "parser/parse_func.h"
#include "parser/parse_node.h"
@@ -58,6 +60,7 @@
#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
#include "rewrite/rewriteSupport.h"
+#include "tcop/tcopprot.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -493,6 +496,68 @@ static void get_reloptions(StringInfo buf, Datum reloptions);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
+/* return the query as postgres will rewrite */
+Datum
+pg_get_query_def(PG_FUNCTION_ARGS)
+{
+ char *sql = TextDatumGetCString(PG_GETARG_TEXT_PP(0));
+ List *parsetree_list;
+ List *querytree_list;
+ RawStmt *parsetree;
+ Query *query;
+ bool snapshot_set = false;
+ StringInfoData buf;
+ StringInfoData res;
+ ListCell *lc;
+
+ parsetree_list = pg_parse_query(sql);
+
+ /* only support one statement at a time */
+ if (list_length(parsetree_list) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("a single statement should be provided")));
+
+ initStringInfo(&res);
+
+ parsetree = linitial_node(RawStmt, parsetree_list);
+
+ /*
+ * Set up a snapshot if parse analysis/planning will need one.
+ */
+ if (analyze_requires_snapshot(parsetree))
+ {
+ PushActiveSnapshot(GetTransactionSnapshot());
+ snapshot_set = true;
+ }
+
+ querytree_list = pg_analyze_and_rewrite(parsetree, sql,
+ NULL, 0, NULL);
+
+ /* Done with the snapshot used for parsing/planning */
+ if (snapshot_set)
+ PopActiveSnapshot();
+
+ foreach(lc, querytree_list)
+ {
+ query = (Query *) lfirst(lc);
+ initStringInfo(&buf);
+
+ if (query->utilityStmt)
+ appendStringInfo(&res, "%s;\n", sql);
+ else
+ {
+ get_query_def(query, &buf, NIL, NULL,
+ PRETTYFLAG_INDENT,
+ WRAP_COLUMN_DEFAULT, 0);
+
+ appendStringInfo(&res, "%s;\n", buf.data);
+ }
+ }
+ pfree(buf.data);
+
+ PG_RETURN_TEXT_P(string_to_text(res.data));
+}
/* ----------
* pg_get_ruledef - Do it all and return a text
@@ -10989,6 +11054,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
if (strcmp(refname, rte->ctename) != 0)
printalias = true;
}
+ else if (rte->rtekind == RTE_SUBQUERY)
+ {
+ /*
+ * For a subquery RTE, always print alias. A user-specified query
+ * should only be valid if an alias is provided, but our view
+ * expansion doesn't generate aliases, so a rewritten query might
+ * not be valid SQL.
+ */
+ printalias = true;
+ }
if (printalias)
appendStringInfo(buf, " %s", quote_identifier(refname));
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0859dc81ca..a2e5de7967 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3704,6 +3704,9 @@
proargtypes => 'oid oid', prosrc => 'oidge' },
# System-view support functions
+{ oid => '9246', descr => 'show a query as rewritten',
+ proname => 'pg_get_query_def', provolatile => 'v', prorettype => 'text',
+ proargtypes => 'text', prosrc => 'pg_get_query_def' },
{ oid => '1573', descr => 'source text of a rule',
proname => 'pg_get_ruledef', provolatile => 's', prorettype => 'text',
proargtypes => 'oid', prosrc => 'pg_get_ruledef' },
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index d652f7b5fb..8a17936a05 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3120,6 +3120,32 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
WHERE sh.slunit = un.un_name;
(1 row)
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+ def
+--------------------------------------------------------
+ SELECT shoename, +
+ sh_avail, +
+ slcolor, +
+ slminlen, +
+ slminlen_cm, +
+ slmaxlen, +
+ slmaxlen_cm, +
+ slunit +
+ FROM ( SELECT sh.shoename, +
+ sh.sh_avail, +
+ sh.slcolor, +
+ sh.slminlen, +
+ (sh.slminlen * un.un_fact) AS slminlen_cm,+
+ sh.slmaxlen, +
+ (sh.slmaxlen * un.un_fact) AS slmaxlen_cm,+
+ sh.slunit +
+ FROM shoe_data sh, +
+ unit un +
+ WHERE (sh.slunit = un.un_name)) shoe; +
+
+(1 row)
+
--
-- check multi-row VALUES in rules
--
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index b732833e63..a10377fcc5 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1012,6 +1012,9 @@ select pg_get_viewdef('shoe'::regclass) as unpretty;
select pg_get_viewdef('shoe'::regclass,true) as pretty;
select pg_get_viewdef('shoe'::regclass,0) as prettier;
+-- test pg_get_query_def()
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
+
--
-- check multi-row VALUES in rules
--
--
2.35.0
út 1. 2. 2022 v 4:38 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
Hi,
On Mon, Jan 31, 2022 at 10:05:44PM +0100, Pavel Stehule wrote:
I don't feel good about forcing an alias. relname doesn't ensure
uniqueness. You can have two views with the same name from different
schemas. Moreover this field is necessary only when a deparsed query is
printed, not always.Yes I agree.
Isn't possible to compute the correct subquery alias in print time when
it
is missing?
Actually I think that the current code already does everything to generate
unique refnames, it's just that they don't get printed for a query after
view
expansions. I modified the patch to simply make sure that an alias is
displayed when it's a subquery and the output using a custom
pg_get_query_def
is like that:# select pg_get_query_def('select * from nsp1.v1');
pg_get_query_def
-------------------------------
SELECT nb +
FROM ( SELECT 1 AS nb) v1;+(1 row)
# select pg_get_query_def('select * from nsp1.v1, nsp2.v1');
pg_get_query_def
-------------------------------
SELECT v1.nb, +
v1_1.nb +
FROM ( SELECT 1 AS nb) v1,+
( SELECT 1 AS nb) v1_1; +(1 row)
I tested your patch, and it looks so it is working without any problem. All
tests passed.
There is just one question. If printalias = true will be active for all
cases or just with some flag?
I didn't find any visible change of this modification without your
function, so maybe it can be active for all cases without any condition.
Regards
Pavel
On Tue, Feb 1, 2022 at 9:08 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
Hi,
Thanks. +1 for this work. Some comments on v3:
1) How about pg_get_rewritten_query()?
2) Docs missing?
3) How about allowing only the users who are explicitly granted to use
this function like pg_log_backend_memory_contexts,
pg_log_query_plan(in discussion), pg_log_backtrace(in discussion)?
4) initStringInfo in the for loop will palloc every time and will leak
the memory. you probably need to do resetStringInfo in the for loop
instead.
+ foreach(lc, querytree_list)
+ {
+ query = (Query *) lfirst(lc);
+ initStringInfo(&buf);
5) I would even suggest using a temp memory context for this function
alone, because it will ensure we dont' leak any memory which probably
parser, analyzer, rewriter would use.
6) Why can't query be for loop variable?
+ Query *query;
7) Why can't the function check for empty query string and emit error
immedeiately (empty string isn't allowed or some other better error
message), rather than depending on the pg_parse_query?
+ parsetree_list = pg_parse_query(sql);
+
+ /* only support one statement at a time */
+ if (list_length(parsetree_list) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("a single statement should be provided")));
8) Show rewritten query given raw query string
+{ oid => '9246', descr => 'show a query as rewritten',
9) Doesn't the input need a ; at the end of query? not sure if the
parser assumes it as provided?
+SELECT pg_get_query_def('SELECT * FROM shoe') as def;
10) For pg_get_viewdef it makes sense to have the test case in
rules.sql, but shouldn't this function be in misc_functions.sql?
11) Missing bump cat version note in the commit message.
12) I'm just thinking adding an extra option to explain, which will
then print the rewritten query in the explain output, would be useful
than having a separate function to do this?
13) Somebody might also be interested to just get the completed
planned query i.e. output of pg_plan_query? or the other way, given
the query plan as input to a function, can we get the query back?
something like postgres_fdw/deparse.c does?Regards,
Bharath Rupireddy.
Hi,
On Wed, Feb 02, 2022 at 07:09:35PM +0530, Bharath Rupireddy wrote:
On Tue, Feb 1, 2022 at 9:08 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
Hi,
Thanks. +1 for this work. Some comments on v3:
1) How about pg_get_rewritten_query()?
Argh, I just realized that I sent the patch from the wrong branch. Per
previous complaint from Tom, I'm not proposing that function anymore (I will
publish an extension for that if the patch gets commits) but only expose
get_query_def().
I'm attaching the correct patch this time, sorry about that.
Attachments:
v3-0001-Expose-get_query_def.patchtext/plain; charset=us-asciiDownload
From 0485ea1b507e8f2f1df782a97f11184276d7fca7 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Tue, 29 Jun 2021 00:07:04 +0800
Subject: [PATCH v3] Expose get_query_def()
This function can be useful for external module, for instance if they want to
display a statement after the rewrite stage.
In order to emit valid SQL, make sure that any subquery RTE comes with an
alias.
Author: Julien Rouhaud
Reviewed-by: Gilles Darold
Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/20210627041138.zklczwmu3ms4ufnk@nol
---
src/backend/utils/adt/ruleutils.c | 15 +++++++++++----
src/include/utils/ruleutils.h | 3 +++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 039b1d2b95..3db2948984 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -389,9 +389,6 @@ static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags, int wrapColumn);
-static void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
- TupleDesc resultDesc,
- int prettyFlags, int wrapColumn, int startIndent);
static void get_values_def(List *values_lists, deparse_context *context);
static void get_with_clause(Query *query, deparse_context *context);
static void get_select_query_def(Query *query, deparse_context *context,
@@ -5344,7 +5341,7 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
* the view represented by a SELECT query.
* ----------
*/
-static void
+void
get_query_def(Query *query, StringInfo buf, List *parentnamespace,
TupleDesc resultDesc,
int prettyFlags, int wrapColumn, int startIndent)
@@ -10989,6 +10986,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
if (strcmp(refname, rte->ctename) != 0)
printalias = true;
}
+ else if (rte->rtekind == RTE_SUBQUERY)
+ {
+ /*
+ * For a subquery RTE, always print alias. A user-specified query
+ * should only be valid if an alias is provided, but our view
+ * expansion doesn't generate aliases, so a rewritten query might
+ * not be valid SQL.
+ */
+ printalias = true;
+ }
if (printalias)
appendStringInfo(buf, " %s", quote_identifier(refname));
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index e8090c96d7..f512bb6867 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -39,6 +39,9 @@ extern List *select_rtable_names_for_explain(List *rtable,
Bitmapset *rels_used);
extern char *generate_collation_name(Oid collid);
extern char *generate_opclass_name(Oid opclass);
+void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
+ TupleDesc resultDesc,
+ int prettyFlags, int wrapColumn, int startIndent);
extern char *get_range_partbound_string(List *bound_datums);
extern char *pg_get_statisticsobjdef_string(Oid statextid);
--
2.35.0
Hi,
On Tue, Feb 01, 2022 at 08:35:00PM +0100, Pavel Stehule wrote:
I tested your patch, and it looks so it is working without any problem. All
tests passed.There is just one question. If printalias = true will be active for all
cases or just with some flag?
Sorry, as I just replied to Bharath I sent the wrong patch. The new patch has
the same modification with printalias = true though, so I can still answer that
question. The change is active for all cases, however it's a no-op for any
in-core case, as a query sent by a client should be valid, and thus should have
an alias attached to all subqueries. It's only different if you call
get_query_def() on the result of pg_analyze_and_rewrite(), since this code
doesn't add the subquery aliases as those aren't needed for the execution part.
st 2. 2. 2022 v 15:18 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
Hi,
On Tue, Feb 01, 2022 at 08:35:00PM +0100, Pavel Stehule wrote:
I tested your patch, and it looks so it is working without any problem.
All
tests passed.
There is just one question. If printalias = true will be active for all
cases or just with some flag?Sorry, as I just replied to Bharath I sent the wrong patch. The new patch
has
the same modification with printalias = true though, so I can still answer
that
question. The change is active for all cases, however it's a no-op for any
in-core case, as a query sent by a client should be valid, and thus should
have
an alias attached to all subqueries. It's only different if you call
get_query_def() on the result of pg_analyze_and_rewrite(), since this code
doesn't add the subquery aliases as those aren't needed for the execution
part.
ok.
I checked this trivial patch, and I don't see any problem. Again I run
check-world with success. The documentation for this feature is not
necessary. But I am not sure about regress tests. Without any other code,
enfosing printalias will be invisible. What do you think about the
transformation of your extension to a new module in src/test/modules? Maybe
it can be used for other checks in future.
Regards
Pavel
Hi,
On Wed, Feb 02, 2022 at 07:49:41PM +0100, Pavel Stehule wrote:
I checked this trivial patch, and I don't see any problem. Again I run
check-world with success. The documentation for this feature is not
necessary. But I am not sure about regress tests. Without any other code,
enfosing printalias will be invisible. What do you think about the
transformation of your extension to a new module in src/test/modules? Maybe
it can be used for other checks in future.
I'm not opposed, but previously Tom explicitly said that he thinks this feature
is useless and is strongly opposed to making any kind of promise that the
current interface to make it possible (if get_query_def() is exposed) would be
maintained. Adding such a test module would probably a reason to reject the
patch altogether. I'm just hoping that this change, which is a no-op for
any legal query, is acceptable. It can only break something if you feed wrong
data to get_query_def(), which would be my problem and not the project's
problem.
pá 4. 2. 2022 v 10:36 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
Hi,
On Wed, Feb 02, 2022 at 07:49:41PM +0100, Pavel Stehule wrote:
I checked this trivial patch, and I don't see any problem. Again I run
check-world with success. The documentation for this feature is not
necessary. But I am not sure about regress tests. Without any other code,
enfosing printalias will be invisible. What do you think about the
transformation of your extension to a new module in src/test/modules?Maybe
it can be used for other checks in future.
I'm not opposed, but previously Tom explicitly said that he thinks this
feature
is useless and is strongly opposed to making any kind of promise that the
current interface to make it possible (if get_query_def() is exposed)
would be
maintained. Adding such a test module would probably a reason to reject
the
patch altogether. I'm just hoping that this change, which is a no-op for
any legal query, is acceptable. It can only break something if you feed
wrong
data to get_query_def(), which would be my problem and not the project's
problem.
ok, I don't have any problem with it. Then there is not necessarily any
change, and I'll mark this patch as ready for committer.
Regards
Pavel
On Fri, Feb 04, 2022 at 12:45:05PM +0100, Pavel Stehule wrote:
ok, I don't have any problem with it. Then there is not necessarily any
change, and I'll mark this patch as ready for committer.
Thanks Pavel!
I also realized that the CF entry description wasn't accurate anymore, so I
also fixed that.
Julien Rouhaud <rjuju123@gmail.com> writes:
I'm attaching the correct patch this time, sorry about that.
While I'm okay with this in principle, as it stands it fails
headerscheck/cpluspluscheck:
$ src/tools/pginclude/headerscheck
In file included from /tmp/headerscheck.Oi8jj3/test.c:2:
./src/include/utils/ruleutils.h:42:35: error: unknown type name 'StringInfo'; did you mean 'String'?
void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
^~~~~~~~~~
String
./src/include/utils/ruleutils.h:43:9: error: unknown type name 'TupleDesc'
TupleDesc resultDesc,
^~~~~~~~~
We could of course add the necessary #include's to ruleutils.h,
but considering that we seem to have been at some pains to minimize
its #include footprint, I'm not really happy with that approach.
I'm inclined to think that maybe a wrapper function with a slightly
simplified interface would be a better way to go. The deparsed string
could just be returned as a palloc'd "char *", unless you have some reason
to need it to be in a StringInfo. I wonder which of the other parameters
really need to be exposed, too. Several of them seem to be more internal
to ruleutils.c than something that outside callers would care to
manipulate. For instance, since struct deparse_namespace isn't exposed,
there really isn't any way to pass anything except NIL for
parentnamespace.
The bigger picture here is that get_query_def's API has changed over time
internally to ruleutils.c, and I kind of expect that that might continue
in future, so having a wrapper function with a more stable API could be a
good thing.
regards, tom lane
On Fri, Mar 25, 2022 at 05:49:04PM -0400, Tom Lane wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
I'm attaching the correct patch this time, sorry about that.
While I'm okay with this in principle, as it stands it fails
headerscheck/cpluspluscheck:$ src/tools/pginclude/headerscheck
In file included from /tmp/headerscheck.Oi8jj3/test.c:2:
./src/include/utils/ruleutils.h:42:35: error: unknown type name 'StringInfo'; did you mean 'String'?
void get_query_def(Query *query, StringInfo buf, List *parentnamespace,
^~~~~~~~~~
String
./src/include/utils/ruleutils.h:43:9: error: unknown type name 'TupleDesc'
TupleDesc resultDesc,
^~~~~~~~~
Ah thanks for the info. I actually never tried headerscheck/cplupluscheck
before.
We could of course add the necessary #include's to ruleutils.h,
but considering that we seem to have been at some pains to minimize
its #include footprint, I'm not really happy with that approach.
I'm inclined to think that maybe a wrapper function with a slightly
simplified interface would be a better way to go. The deparsed string
could just be returned as a palloc'd "char *", unless you have some reason
to need it to be in a StringInfo. I wonder which of the other parameters
really need to be exposed, too. Several of them seem to be more internal
to ruleutils.c than something that outside callers would care to
manipulate. For instance, since struct deparse_namespace isn't exposed,
there really isn't any way to pass anything except NIL for
parentnamespace.The bigger picture here is that get_query_def's API has changed over time
internally to ruleutils.c, and I kind of expect that that might continue
in future, so having a wrapper function with a more stable API could be a
good thing.
Fair point. That's a much better approach and goes well with the rest of the
exposed functions in that file. I went with a pg_get_querydef, getting rid of
the StringInfo and the List and using the same "bool pretty" flag as used
elsewhere. While doing so, I saw that there were a lot of copy/pasted code for
the pretty flags, so I added a GET_PRETTY_FLAGS(pretty) macro to avoid adding
yet another occurrence. I also kept the wrapColument and startIdent as they
can be easily used by callers.
Attachments:
v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patchtext/plain; charset=us-asciiDownload
From ca49e5e96fcab75c8e19a42d701b16ac96ee9d43 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Tue, 29 Jun 2021 00:07:04 +0800
Subject: [PATCH v4] Add a pg_get_query_def() wrapper around get_query_def().
This function can be useful for external modules, for instance if they want to
display a statement after the rewrite stage.
In order to emit valid SQL, make sure that any subquery RTE comes with an
alias. This is always the case for user facing queries, as the parser will
refuse a subquery without an alias, but that may not be the case for a Query
after rewriting for instance.
Catversion is bumped.
Author: Julien Rouhaud
Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/20210627041138.zklczwmu3ms4ufnk@nol
---
src/backend/utils/adt/ruleutils.c | 49 ++++++++++++++++++++++++-------
src/include/utils/ruleutils.h | 2 ++
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 7f4f3f7369..c9a1dde65f 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -90,6 +90,8 @@
#define PRETTYFLAG_INDENT 0x0002
#define PRETTYFLAG_SCHEMA 0x0004
+#define GET_PRETTY_FLAGS(pretty) ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT)
+
/* Default line length for pretty-print wrapping: 0 means wrap always */
#define WRAP_COLUMN_DEFAULT 0
@@ -526,7 +528,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_ruledef_worker(ruleoid, prettyFlags);
@@ -647,7 +649,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
@@ -667,7 +669,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
char *res;
/* calling this implies we want pretty printing */
- prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
+ prettyFlags = GET_PRETTY_FLAGS(true);
res = pg_get_viewdef_worker(viewoid, prettyFlags, wrap);
@@ -713,7 +715,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
Oid viewoid;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
/* Look up view name. Can't lock it - we might not have privileges. */
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -1056,7 +1058,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = true;
- context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ context.prettyFlags = GET_PRETTY_FLAGS(pretty);
context.wrapColumn = WRAP_COLUMN_DEFAULT;
context.indentLevel = PRETTYINDENT_STD;
context.special_exprkind = EXPR_KIND_NONE;
@@ -1146,7 +1148,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_indexdef_worker(indexrelid, colno, NULL,
colno != 0, false,
@@ -1179,7 +1181,7 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
{
int prettyFlags;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
return pg_get_indexdef_worker(indexrelid, 0, NULL,
true, true,
@@ -1187,6 +1189,23 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
prettyFlags, false);
}
+/* Public wraper around get_query_def */
+char *
+pg_get_querydef(Query *query, bool pretty, int wrapColumn, int startIndent)
+{
+ StringInfoData buf;
+ int prettyFlags;
+
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
+
+ initStringInfo(&buf);
+
+ get_query_def(query, &buf, NIL, NULL, prettyFlags, wrapColumn,
+ startIndent);
+
+ return buf.data;
+}
+
/*
* Internal workhorse to decompile an index definition.
*
@@ -1842,7 +1861,7 @@ pg_get_partkeydef_columns(Oid relid, bool pretty)
{
int prettyFlags;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
return pg_get_partkeydef_worker(relid, prettyFlags, true, false);
}
@@ -2089,7 +2108,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
@@ -2619,7 +2638,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *relname;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
if (OidIsValid(relid))
{
@@ -11002,6 +11021,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
if (strcmp(refname, rte->ctename) != 0)
printalias = true;
}
+ else if (rte->rtekind == RTE_SUBQUERY)
+ {
+ /*
+ * For a subquery RTE, always print alias. A user-specified query
+ * should only be valid if an alias is provided, but our view
+ * expansion doesn't generate aliases, so a rewritten query might
+ * not be valid SQL.
+ */
+ printalias = true;
+ }
if (printalias)
appendStringInfo(buf, " %s", quote_identifier(refname));
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index e8090c96d7..b3767a0497 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -23,6 +23,8 @@ struct PlannedStmt;
extern char *pg_get_indexdef_string(Oid indexrelid);
extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty);
+extern char *pg_get_querydef(Query *query, bool pretty, int wrapColumn,
+ int startIndent);
extern char *pg_get_partkeydef_columns(Oid relid, bool pretty);
extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname);
--
2.33.1
Julien Rouhaud <rjuju123@gmail.com> writes:
[ v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patch ]
This seems about ready to go to me, except for
(1) as an exported API, pg_get_querydef needs a full API spec in its
header comment. "Read the code to figure out what to do" is not OK
in my book.
(2) I don't think this has been thought out too well:
I also kept the wrapColument and startIdent as they
can be easily used by callers.
The appropriate values for these are determined by macros that are
local in ruleutils.c, so it's not that "easy" for outside callers
to conform to standard practice. I suppose we could move
WRAP_COLUMN_DEFAULT etc into ruleutils.h, but is there actually a
use-case for messing with those? I don't see any other exported
functions that go beyond offering a "bool pretty" option, so
I think it might be a mistake for this one to be different.
(The pattern that I see is that a ruleutils function could have
"bool pretty", or it could have "int prettyFlags, int startIndent"
which is an expansion of that; but mixing those levels of detail
doesn't seem very wise.)
regards, tom lane
On Sun, Mar 27, 2022 at 11:53:58AM -0400, Tom Lane wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
[ v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patch ]
This seems about ready to go to me, except for
(1) as an exported API, pg_get_querydef needs a full API spec in its
header comment. "Read the code to figure out what to do" is not OK
in my book.
Fixed.
(2) I don't think this has been thought out too well:
I also kept the wrapColument and startIdent as they
can be easily used by callers.The appropriate values for these are determined by macros that are
local in ruleutils.c, so it's not that "easy" for outside callers
to conform to standard practice. I suppose we could move
WRAP_COLUMN_DEFAULT etc into ruleutils.h, but is there actually a
use-case for messing with those?
As far as I can see the wrapColumn and startIndent are independant of the
pretty flags, and don't really have magic numbers for those
(WRAP_COLUMN_DEFAULT sure exists, but the value isn't really surprising).
I don't see any other exported
functions that go beyond offering a "bool pretty" option, so
I think it might be a mistake for this one to be different.
There's the SQL function pg_get_viewdef_wrap() that accept a custom wrapColumn.
That being said I'm totally ok with just exposing a "pretty" parameter and use
WRAP_COLUMN_DEFAULT. In any case I agree that exposing startIndent doesn't
serve any purpose.
I'm attaching a v5 with hopefully a better comment for the function, and only
the "pretty" parameter.
Attachments:
v5-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patchtext/plain; charset=us-asciiDownload
From 6497eb63cecdccc2669ec4ff316666ab24f6f3e1 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Tue, 29 Jun 2021 00:07:04 +0800
Subject: [PATCH v5] Add a pg_get_query_def() wrapper around get_query_def().
This function can be useful for external modules, for instance if they want to
display a statement after the rewrite stage.
In order to emit valid SQL, make sure that any subquery RTE comes with an
alias. This is always the case for user facing queries, as the parser will
refuse a subquery without an alias, but that may not be the case for a Query
after rewriting for instance.
Catversion is bumped.
Author: Julien Rouhaud
Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/20210627041138.zklczwmu3ms4ufnk@nol
---
src/backend/utils/adt/ruleutils.c | 55 +++++++++++++++++++++++++------
src/include/utils/ruleutils.h | 1 +
2 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index df5c486501..60fb167067 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -90,6 +90,8 @@
#define PRETTYFLAG_INDENT 0x0002
#define PRETTYFLAG_SCHEMA 0x0004
+#define GET_PRETTY_FLAGS(pretty) ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT)
+
/* Default line length for pretty-print wrapping: 0 means wrap always */
#define WRAP_COLUMN_DEFAULT 0
@@ -532,7 +534,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_ruledef_worker(ruleoid, prettyFlags);
@@ -653,7 +655,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
@@ -673,7 +675,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
char *res;
/* calling this implies we want pretty printing */
- prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
+ prettyFlags = GET_PRETTY_FLAGS(true);
res = pg_get_viewdef_worker(viewoid, prettyFlags, wrap);
@@ -719,7 +721,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
Oid viewoid;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
/* Look up view name. Can't lock it - we might not have privileges. */
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -1062,7 +1064,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
context.windowClause = NIL;
context.windowTList = NIL;
context.varprefix = true;
- context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ context.prettyFlags = GET_PRETTY_FLAGS(pretty);
context.wrapColumn = WRAP_COLUMN_DEFAULT;
context.indentLevel = PRETTYINDENT_STD;
context.special_exprkind = EXPR_KIND_NONE;
@@ -1152,7 +1154,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_indexdef_worker(indexrelid, colno, NULL,
colno != 0, false,
@@ -1185,7 +1187,7 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
{
int prettyFlags;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
return pg_get_indexdef_worker(indexrelid, 0, NULL,
true, true,
@@ -1193,6 +1195,29 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
prettyFlags, false);
}
+/* ----------
+ * pg_get_querydef
+ *
+ * Public wrapper around get_query_def to parse back one query parsetree. The
+ * pretty flags are determined by GET_PRETTY_FLAGS(pretty).
+ *
+ * The result is a palloc'd C string.
+ */
+char *
+pg_get_querydef(Query *query, bool pretty)
+{
+ StringInfoData buf;
+ int prettyFlags;
+
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
+
+ initStringInfo(&buf);
+
+ get_query_def(query, &buf, NIL, NULL, prettyFlags, WRAP_COLUMN_DEFAULT, 0);
+
+ return buf.data;
+}
+
/*
* Internal workhorse to decompile an index definition.
*
@@ -1848,7 +1873,7 @@ pg_get_partkeydef_columns(Oid relid, bool pretty)
{
int prettyFlags;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
return pg_get_partkeydef_worker(relid, prettyFlags, true, false);
}
@@ -2095,7 +2120,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *res;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
@@ -2625,7 +2650,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
int prettyFlags;
char *relname;
- prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
+ prettyFlags = GET_PRETTY_FLAGS(pretty);
if (OidIsValid(relid))
{
@@ -11218,6 +11243,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
if (strcmp(refname, rte->ctename) != 0)
printalias = true;
}
+ else if (rte->rtekind == RTE_SUBQUERY)
+ {
+ /*
+ * For a subquery RTE, always print alias. A user-specified query
+ * should only be valid if an alias is provided, but our view
+ * expansion doesn't generate aliases, so a rewritten query might
+ * not be valid SQL.
+ */
+ printalias = true;
+ }
if (printalias)
appendStringInfo(buf, " %s", quote_identifier(refname));
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index e8090c96d7..7d489718a3 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -23,6 +23,7 @@ struct PlannedStmt;
extern char *pg_get_indexdef_string(Oid indexrelid);
extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty);
+extern char *pg_get_querydef(Query *query, bool pretty);
extern char *pg_get_partkeydef_columns(Oid relid, bool pretty);
extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname);
--
2.33.1
Julien Rouhaud <rjuju123@gmail.com> writes:
I'm attaching a v5 with hopefully a better comment for the function, and only
the "pretty" parameter.
Pushed with some minor cosmetic adjustments.
regards, tom lane
On Mon, Mar 28, 2022 at 11:20:42AM -0400, Tom Lane wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
I'm attaching a v5 with hopefully a better comment for the function, and only
the "pretty" parameter.Pushed with some minor cosmetic adjustments.
Thanks a lot!
I just published an extension using this for the use case I'm interested in.