Re: Deparsing rewritten query

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, 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-03-25 21:49:04
Message-ID: 3481161.1648244944@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Julien Rouhaud <rjuju123(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2022-03-25 21:52:03 Re: pgsql: Add 'basebackup_to_shell' contrib module.
Previous Message Daniel Gustafsson 2022-03-25 21:48:33 Re: [PATCH] Enable SSL library detection via PQsslAttribute