pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedDeparsing rewritten query
24+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2022-03-28 d22646922d Add public ruleutils.c entry point to deparse a Query.
* Deparsing rewritten query
@ 2021-06-27 04:11 Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 04:31 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 24+ messages in thread
From: Julien Rouhaud @ 2021-06-27 04:11 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
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.patch (text/x-diff, 6.4K) download
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
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2021-06-27 04:31 ` Pavel Stehule <pavel.stehule@gmail.com>
1 sibling, 0 replies; 24+ messages in thread
From: Pavel Stehule @ 2021-06-27 04:31 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2021-06-27 14:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
1 sibling, 1 reply; 24+ messages in thread
From: Tom Lane @ 2021-06-27 14:34 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-27 15:03 ` Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2021-06-27 15:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2021-06-27 15:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:21 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
0 siblings, 2 replies; 24+ messages in thread
From: Tom Lane @ 2021-06-27 15:14 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-27 15:21 ` Julien Rouhaud <rjuju123@gmail.com>
1 sibling, 0 replies; 24+ messages in thread
From: Julien Rouhaud @ 2021-06-27 15:21 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@lists.postgresql.org
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-06-28 14:06 ` Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
1 sibling, 1 reply; 24+ messages in thread
From: Gilles Darold @ 2021-06-28 14:06 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
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/
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
@ 2021-06-28 16:41 ` Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2021-06-28 16:41 UTC (permalink / raw)
To: Gilles Darold <gilles@darold.net>; +Cc: pgsql-hackers@lists.postgresql.org
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.patch (text/x-diff, 3.1K) download
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
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2021-06-28 17:30 ` Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Gilles Darold @ 2021-06-28 17:30 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
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/
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
@ 2022-01-31 17:46 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Pavel Stehule @ 2022-01-31 17:46 UTC (permalink / raw)
To: Gilles Darold <gilles@darold.net>; +Cc: Julien Rouhaud <rjuju123@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
>
> 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/
>
>
>
>
>
>
>
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
@ 2022-01-31 18:09 ` Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2022-01-31 18:09 UTC (permalink / raw)
To: Pavel Stehule <pavel.stehule@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-01-31 21:05 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Pavel Stehule @ 2022-01-31 21:05 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
@ 2022-02-01 03:37 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 13:39 ` Re: Deparsing rewritten query Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
0 siblings, 2 replies; 24+ messages in thread
From: Julien Rouhaud @ 2022-02-01 03:37 UTC (permalink / raw)
To: Pavel Stehule <pavel.stehule@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.patch (text/plain, 6.2K) download
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
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-02-01 19:35 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
1 sibling, 1 reply; 24+ messages in thread
From: Pavel Stehule @ 2022-02-01 19:35 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
ú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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
@ 2022-02-02 14:16 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 18:49 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2022-02-02 14:16 UTC (permalink / raw)
To: Pavel Stehule <pavel.stehule@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-02-02 18:49 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-04 09:35 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Pavel Stehule @ 2022-02-02 18:49 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 18:49 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
@ 2022-02-04 09:35 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-04 11:45 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2022-02-04 09:35 UTC (permalink / raw)
To: Pavel Stehule <pavel.stehule@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 18:49 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-04 09:35 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-02-04 11:45 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-05 05:04 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Pavel Stehule @ 2022-02-04 11:45 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 18:49 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-04 09:35 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-04 11:45 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
@ 2022-02-05 05:04 ` Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 0 replies; 24+ messages in thread
From: Julien Rouhaud @ 2022-02-05 05:04 UTC (permalink / raw)
To: Pavel Stehule <pavel.stehule@gmail.com>; +Cc: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-02-02 13:39 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2022-02-02 14:12 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
1 sibling, 1 reply; 24+ messages in thread
From: Bharath Rupireddy @ 2022-02-02 13:39 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Pavel Stehule <pavel.stehule@gmail.com>; Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 13:39 ` Re: Deparsing rewritten query Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
@ 2022-02-02 14:12 ` Julien Rouhaud <rjuju123@gmail.com>
2022-03-25 21:49 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 24+ messages in thread
From: Julien Rouhaud @ 2022-02-02 14:12 UTC (permalink / raw)
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; +Cc: Pavel Stehule <pavel.stehule@gmail.com>; Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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.patch (text/plain, 3.1K) download
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
* Re: Deparsing rewritten query
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 14:34 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Re: Deparsing rewritten query Tom Lane <tgl@sss.pgh.pa.us>
2021-06-28 14:06 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Re: Deparsing rewritten query Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Re: Deparsing rewritten query Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 13:39 ` Re: Deparsing rewritten query Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2022-02-02 14:12 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
@ 2022-03-25 21:49 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 24+ messages in thread
From: Tom Lane @ 2022-03-25 21:49 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>; Pavel Stehule <pavel.stehule@gmail.com>; Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
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
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Deparsing rewritten query
@ 2021-06-27 13:06 Ranier Vilela <ranier.vf@gmail.com>
2021-06-27 13:35 ` Re: Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 24+ messages in thread
From: Ranier Vilela @ 2021-06-27 13:06 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Julien Rouhaud <rjuju123@gmail.com>; Pavel Stehule <pavel.stehule@gmail.com>
> 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.patch (application/octet-stream, 6.4K) download
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
* Re: Deparsing rewritten query
2021-06-27 13:06 Re: Deparsing rewritten query Ranier Vilela <ranier.vf@gmail.com>
@ 2021-06-27 13:35 ` Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 0 replies; 24+ messages in thread
From: Julien Rouhaud @ 2021-06-27 13:35 UTC (permalink / raw)
To: Ranier Vilela <ranier.vf@gmail.com>; +Cc: pgsql-hackers; Pavel Stehule <pavel.stehule@gmail.com>
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.
^ permalink raw reply [nested|flat] 24+ messages in thread
end of thread, other threads:[~2022-03-25 21:49 UTC | newest]
Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-06-27 04:11 Deparsing rewritten query Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 04:31 ` Pavel Stehule <pavel.stehule@gmail.com>
2021-06-27 14:34 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:03 ` Julien Rouhaud <rjuju123@gmail.com>
2021-06-27 15:14 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 15:21 ` Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 14:06 ` Gilles Darold <gilles@darold.net>
2021-06-28 16:41 ` Julien Rouhaud <rjuju123@gmail.com>
2021-06-28 17:30 ` Gilles Darold <gilles@darold.net>
2022-01-31 17:46 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-01-31 18:09 ` Julien Rouhaud <rjuju123@gmail.com>
2022-01-31 21:05 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-01 03:37 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-01 19:35 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-02 14:16 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 18:49 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-04 09:35 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-04 11:45 ` Pavel Stehule <pavel.stehule@gmail.com>
2022-02-05 05:04 ` Julien Rouhaud <rjuju123@gmail.com>
2022-02-02 13:39 ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2022-02-02 14:12 ` Julien Rouhaud <rjuju123@gmail.com>
2022-03-25 21:49 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-06-27 13:06 Re: Deparsing rewritten query Ranier Vilela <ranier.vf@gmail.com>
2021-06-27 13:35 ` Julien Rouhaud <rjuju123@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox