On Fri, Mar 25, 2022 at 05:49:04PM -0400, Tom Lane wrote: > Julien Rouhaud <rjuju...@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.
>From ca49e5e96fcab75c8e19a42d701b16ac96ee9d43 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud <julien.rouh...@free.fr> Date: Tue, 29 Jun 2021 00:07:04 +0800 Subject: [PATCH v4] Add a pg_get_query_def() wrapper around get_query_def(). This function can be useful for external modules, for instance if they want to display a statement after the rewrite stage. In order to emit valid SQL, make sure that any subquery RTE comes with an alias. This is always the case for user facing queries, as the parser will refuse a subquery without an alias, but that may not be the case for a Query after rewriting for instance. Catversion is bumped. Author: Julien Rouhaud Reviewed-by: Pavel Stehule Discussion: https://postgr.es/m/20210627041138.zklczwmu3ms4ufnk@nol --- src/backend/utils/adt/ruleutils.c | 49 ++++++++++++++++++++++++------- src/include/utils/ruleutils.h | 2 ++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 7f4f3f7369..c9a1dde65f 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -90,6 +90,8 @@ #define PRETTYFLAG_INDENT 0x0002 #define PRETTYFLAG_SCHEMA 0x0004 +#define GET_PRETTY_FLAGS(pretty) ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT) + /* Default line length for pretty-print wrapping: 0 means wrap always */ #define WRAP_COLUMN_DEFAULT 0 @@ -526,7 +528,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS) int prettyFlags; char *res; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); res = pg_get_ruledef_worker(ruleoid, prettyFlags); @@ -647,7 +649,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS) int prettyFlags; char *res; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT); @@ -667,7 +669,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS) char *res; /* calling this implies we want pretty printing */ - prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA; + prettyFlags = GET_PRETTY_FLAGS(true); res = pg_get_viewdef_worker(viewoid, prettyFlags, wrap); @@ -713,7 +715,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS) Oid viewoid; char *res; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); /* Look up view name. Can't lock it - we might not have privileges. */ viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname)); @@ -1056,7 +1058,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) context.windowClause = NIL; context.windowTList = NIL; context.varprefix = true; - context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + context.prettyFlags = GET_PRETTY_FLAGS(pretty); context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = PRETTYINDENT_STD; context.special_exprkind = EXPR_KIND_NONE; @@ -1146,7 +1148,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS) int prettyFlags; char *res; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); res = pg_get_indexdef_worker(indexrelid, colno, NULL, colno != 0, false, @@ -1179,7 +1181,7 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty) { int prettyFlags; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); return pg_get_indexdef_worker(indexrelid, 0, NULL, true, true, @@ -1187,6 +1189,23 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty) prettyFlags, false); } +/* Public wraper around get_query_def */ +char * +pg_get_querydef(Query *query, bool pretty, int wrapColumn, int startIndent) +{ + StringInfoData buf; + int prettyFlags; + + prettyFlags = GET_PRETTY_FLAGS(pretty); + + initStringInfo(&buf); + + get_query_def(query, &buf, NIL, NULL, prettyFlags, wrapColumn, + startIndent); + + return buf.data; +} + /* * Internal workhorse to decompile an index definition. * @@ -1842,7 +1861,7 @@ pg_get_partkeydef_columns(Oid relid, bool pretty) { int prettyFlags; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); return pg_get_partkeydef_worker(relid, prettyFlags, true, false); } @@ -2089,7 +2108,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS) int prettyFlags; char *res; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true); @@ -2619,7 +2638,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS) int prettyFlags; char *relname; - prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + prettyFlags = GET_PRETTY_FLAGS(pretty); if (OidIsValid(relid)) { @@ -11002,6 +11021,16 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) if (strcmp(refname, rte->ctename) != 0) printalias = true; } + else if (rte->rtekind == RTE_SUBQUERY) + { + /* + * For a subquery RTE, always print alias. A user-specified query + * should only be valid if an alias is provided, but our view + * expansion doesn't generate aliases, so a rewritten query might + * not be valid SQL. + */ + printalias = true; + } if (printalias) appendStringInfo(buf, " %s", quote_identifier(refname)); diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h index e8090c96d7..b3767a0497 100644 --- a/src/include/utils/ruleutils.h +++ b/src/include/utils/ruleutils.h @@ -23,6 +23,8 @@ struct PlannedStmt; extern char *pg_get_indexdef_string(Oid indexrelid); extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty); +extern char *pg_get_querydef(Query *query, bool pretty, int wrapColumn, + int startIndent); extern char *pg_get_partkeydef_columns(Oid relid, bool pretty); extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname); -- 2.33.1
