Re: Deparsing rewritten query

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Gilles Darold <gilles(at)darold(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Deparsing rewritten query
Date: 2022-02-02 13:39:35
Message-ID: CALj2ACVx5OAEW_+azogOR8qaYN2sH4jDV-ZTxY+xuDsd3KRhCw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 1, 2022 at 9:08 AM Julien Rouhaud <rjuju123(at)gmail(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2022-02-02 14:08:52 Re: Schema variables - new implementation for Postgres 15
Previous Message Dilip Kumar 2022-02-02 13:39:15 Re: Make relfile tombstone files conditional on WAL level