From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
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: | Add a pg_get_query_def function (was Re: Deparsing rewritten query) |
Date: | 2022-03-27 05:21:09 |
Message-ID: | 20220327052109.phgehhkrvgtz4bsz@jrouhaud |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 25, 2022 at 05:49:04PM -0400, Tom Lane wrote:
> 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,
> ^~~~~~~~~
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.
Attachment | Content-Type | Size |
---|---|---|
v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patch | text/plain | 6.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tatsuo Ishii | 2022-03-27 06:28:41 | Re: [HACKERS] WIP aPatch: Pgbench Serialization and deadlock errors |
Previous Message | Julien Rouhaud | 2022-03-27 04:15:25 | Re: make MaxBackends available in _PG_init |