On Fri, Mar 25, 2022 at 05:49:04PM -0400, Tom Lane wrote:
> 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,
> ^~~~~~~~~
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.
