From: Tom Lane <tgl@sss.pgh.pa.us>
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>
Subject: Re: Deparsing rewritten query
In-reply-to: <20220202141231.rlogsu3t33ky6db7@jrouhaud>
References: <20210627150343.zslwnw7g564f2zz4@nol>
 <8085.1624806845@sss.pgh.pa.us>
 <8f0b88a8-e9b8-0fd8-4dd1-ad14c4fdca0e@darold.net>
 <20210628164147.vyglw4rbyyypncqf@nol>
 <44a01de0-7c46-1ea9-41b0-c85a795c3c6c@darold.net>
 <CAFj8pRCzdbPJkUOP6e-mci=Vo91FqBOLvYsiJWAaVkEXGyJb5Q@mail.gmail.com>
 <20220131180938.id2jjihudd4xmwjh@jrouhaud>
 <CAFj8pRBb2bDUqgzv_r1LVUNo6CEQsEA_9NQDTrrk12a5S=EWJA@mail.gmail.com>
 <20220201033707.facusnf6evjmnoo3@nol>
 <CALj2ACVx5OAEW_+azogOR8qaYN2sH4jDV-ZTxY+xuDsd3KRhCw@mail.gmail.com>
 <20220202141231.rlogsu3t33ky6db7@jrouhaud>
Comments: In-reply-to Julien Rouhaud <rjuju123@gmail.com>
	message dated "Wed, 02 Feb 2022 22:12:31 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3481160.1648244944.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 25 Mar 2022 17:49:04 -0400
Message-ID: <3481161.1648244944@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3481161.1648244944%40sss.pgh.pa.us>
Precedence: bulk

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 'StringInf=
o'; 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