On Sun, Mar 27, 2022 at 11:53:58AM -0400, Tom Lane wrote: > Julien Rouhaud <rjuju...@gmail.com> writes: > > [ v4-0001-Add-a-pg_get_query_def-wrapper-around-get_query_d.patch ] > > This seems about ready to go to me, except for > > (1) as an exported API, pg_get_querydef needs a full API spec in its > header comment. "Read the code to figure out what to do" is not OK > in my book.
Fixed. > (2) I don't think this has been thought out too well: > > > I also kept the wrapColument and startIdent as they > > can be easily used by callers. > > The appropriate values for these are determined by macros that are > local in ruleutils.c, so it's not that "easy" for outside callers > to conform to standard practice. I suppose we could move > WRAP_COLUMN_DEFAULT etc into ruleutils.h, but is there actually a > use-case for messing with those? As far as I can see the wrapColumn and startIndent are independant of the pretty flags, and don't really have magic numbers for those (WRAP_COLUMN_DEFAULT sure exists, but the value isn't really surprising). > I don't see any other exported > functions that go beyond offering a "bool pretty" option, so > I think it might be a mistake for this one to be different. There's the SQL function pg_get_viewdef_wrap() that accept a custom wrapColumn. That being said I'm totally ok with just exposing a "pretty" parameter and use WRAP_COLUMN_DEFAULT. In any case I agree that exposing startIndent doesn't serve any purpose. I'm attaching a v5 with hopefully a better comment for the function, and only the "pretty" parameter.
>From 6497eb63cecdccc2669ec4ff316666ab24f6f3e1 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 v5] 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 | 55 +++++++++++++++++++++++++------ src/include/utils/ruleutils.h | 1 + 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index df5c486501..60fb167067 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 @@ -532,7 +534,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); @@ -653,7 +655,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); @@ -673,7 +675,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); @@ -719,7 +721,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)); @@ -1062,7 +1064,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; @@ -1152,7 +1154,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, @@ -1185,7 +1187,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, @@ -1193,6 +1195,29 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty) prettyFlags, false); } +/* ---------- + * pg_get_querydef + * + * Public wrapper around get_query_def to parse back one query parsetree. The + * pretty flags are determined by GET_PRETTY_FLAGS(pretty). + * + * The result is a palloc'd C string. + */ +char * +pg_get_querydef(Query *query, bool pretty) +{ + StringInfoData buf; + int prettyFlags; + + prettyFlags = GET_PRETTY_FLAGS(pretty); + + initStringInfo(&buf); + + get_query_def(query, &buf, NIL, NULL, prettyFlags, WRAP_COLUMN_DEFAULT, 0); + + return buf.data; +} + /* * Internal workhorse to decompile an index definition. * @@ -1848,7 +1873,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); } @@ -2095,7 +2120,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); @@ -2625,7 +2650,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)) { @@ -11218,6 +11243,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..7d489718a3 100644 --- a/src/include/utils/ruleutils.h +++ b/src/include/utils/ruleutils.h @@ -23,6 +23,7 @@ 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); extern char *pg_get_partkeydef_columns(Oid relid, bool pretty); extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname); -- 2.33.1
