On 27 April 2016 at 08:36, Dean Rasheed <dean.a.rash...@gmail.com> wrote: > Here is a rough patch based on the way pg_dump handles this. It still > needs a bit of polishing -- in particular I think fmtReloptionsArray() > (copied from pg_dump) should probably be moved to string_utils.c so > that it can be shared between pg_dump and psql. Also, I'm not sure > that's the best name for it -- I think appendReloptionsArray() is a > more accurate description of what is does. >
Here are updated patches doing that --- the first moves fmtReloptionsArray() from pg_dump.c to fe_utils/string_utils.c so that it can be shared by pg_dump and psql, and renames it to appendReloptionsArray(). The second patch fixes the actual psql bug. Regards, Dean
From a01c897bf2945f8ebef39bf3c58bb14e1739abf9 Mon Sep 17 00:00:00 2001 From: Dean Rasheed <dean.a.rash...@gmail.com> Date: Mon, 2 May 2016 11:27:15 +0100 Subject: [PATCH 1/2] Move and rename fmtReloptionsArray(). Move fmtReloptionsArray() from pg_dump.c to string_utils.c so that it is available to other frontend code. In particular psql's \ev and \sv commands need it to handle view reloptions. Also rename the function to appendReloptionsArray(), which is a more accurate description of what it does. --- src/bin/pg_dump/pg_backup.h | 7 ++++ src/bin/pg_dump/pg_dump.c | 80 +++---------------------------------- src/fe_utils/string_utils.c | 72 +++++++++++++++++++++++++++++++++ src/include/fe_utils/string_utils.h | 3 ++ 4 files changed, 88 insertions(+), 74 deletions(-) diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 83f6029..6b162a4 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -288,4 +288,11 @@ extern int archprintf(Archive *AH, const char *fmt,...) pg_attribute_printf(2, 3 #define appendStringLiteralAH(buf,str,AH) \ appendStringLiteral(buf, str, (AH)->encoding, (AH)->std_strings) +#define appendReloptionsArrayAH(buf,reloptions,prefix,AH) \ + do { \ + if (!appendReloptionsArray(buf, reloptions, prefix, \ + (AH)->encoding, (AH)->std_strings)) \ + write_msg(NULL, "WARNING: could not parse reloptions array\n"); \ + } while (0) + #endif /* PG_BACKUP_H */ diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d3f5157..3005bf5 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -261,8 +261,6 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, static const char *getAttrName(int attrnum, TableInfo *tblInfo); static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer); static bool nonemptyReloptions(const char *reloptions); -static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, - const char *reloptions, const char *prefix); static char *get_synchronized_snapshot(Archive *fout); static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query); static void setupDumpWorker(Archive *AHX); @@ -15046,7 +15044,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (nonemptyReloptions(tbinfo->reloptions)) { appendPQExpBufferStr(q, " WITH ("); - fmtReloptionsArray(fout, q, tbinfo->reloptions, ""); + appendReloptionsArrayAH(q, tbinfo->reloptions, "", fout); appendPQExpBufferChar(q, ')'); } result = createViewAsClause(fout, tbinfo); @@ -15301,13 +15299,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (nonemptyReloptions(tbinfo->reloptions)) { addcomma = true; - fmtReloptionsArray(fout, q, tbinfo->reloptions, ""); + appendReloptionsArrayAH(q, tbinfo->reloptions, "", fout); } if (nonemptyReloptions(tbinfo->toast_reloptions)) { if (addcomma) appendPQExpBufferStr(q, ", "); - fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast."); + appendReloptionsArrayAH(q, tbinfo->toast_reloptions, "toast.", + fout); } appendPQExpBufferChar(q, ')'); } @@ -15908,7 +15907,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) if (nonemptyReloptions(indxinfo->indreloptions)) { appendPQExpBufferStr(q, " WITH ("); - fmtReloptionsArray(fout, q, indxinfo->indreloptions, ""); + appendReloptionsArrayAH(q, indxinfo->indreloptions, "", fout); appendPQExpBufferChar(q, ')'); } @@ -16809,7 +16808,7 @@ dumpRule(Archive *fout, RuleInfo *rinfo) { appendPQExpBuffer(cmd, "ALTER VIEW %s SET (", fmtId(tbinfo->dobj.name)); - fmtReloptionsArray(fout, cmd, rinfo->reloptions, ""); + appendReloptionsArrayAH(cmd, rinfo->reloptions, "", fout); appendPQExpBufferStr(cmd, ");\n"); } @@ -17704,73 +17703,6 @@ nonemptyReloptions(const char *reloptions) } /* - * Format a reloptions array and append it to the given buffer. - * - * "prefix" is prepended to the option names; typically it's "" or "toast.". - * - * Note: this logic should generally match the backend's flatten_reloptions() - * (in adt/ruleutils.c). - */ -static void -fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions, - const char *prefix) -{ - char **options; - int noptions; - int i; - - if (!parsePGArray(reloptions, &options, &noptions)) - { - write_msg(NULL, "WARNING: could not parse reloptions array\n"); - if (options) - free(options); - return; - } - - for (i = 0; i < noptions; i++) - { - char *option = options[i]; - char *name; - char *separator; - char *value; - - /* - * Each array element should have the form name=value. If the "=" is - * missing for some reason, treat it like an empty value. - */ - name = option; - separator = strchr(option, '='); - if (separator) - { - *separator = '\0'; - value = separator + 1; - } - else - value = ""; - - if (i > 0) - appendPQExpBufferStr(buffer, ", "); - appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name)); - - /* - * In general we need to quote the value; but to avoid unnecessary - * clutter, do not quote if it is an identifier that would not need - * quoting. (We could also allow numbers, but that is a bit trickier - * than it looks --- for example, are leading zeroes significant? We - * don't want to assume very much here about what custom reloptions - * might mean.) - */ - if (strcmp(fmtId(value), value) == 0) - appendPQExpBufferStr(buffer, value); - else - appendStringLiteralAH(buffer, value, fout); - } - - if (options) - free(options); -} - -/* * Execute an SQL query and verify that we got exactly one row back. */ static PGresult * diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c index c57d836..aeef12c 100644 --- a/src/fe_utils/string_utils.c +++ b/src/fe_utils/string_utils.c @@ -462,6 +462,78 @@ parsePGArray(const char *atext, char ***itemarray, int *nitems) /* + * Format a reloptions array and append it to the given buffer. + * + * "prefix" is prepended to the option names; typically it's "" or "toast.". + * + * Returns false if the reloptions array could not be parsed (in which case + * nothing will have been appended to the buffer), or true on success. + * + * Note: this logic should generally match the backend's flatten_reloptions() + * (in adt/ruleutils.c). + */ +bool +appendReloptionsArray(PQExpBuffer buffer, const char *reloptions, + const char *prefix, int encoding, bool std_strings) +{ + char **options; + int noptions; + int i; + + if (!parsePGArray(reloptions, &options, &noptions)) + { + if (options) + free(options); + return false; + } + + for (i = 0; i < noptions; i++) + { + char *option = options[i]; + char *name; + char *separator; + char *value; + + /* + * Each array element should have the form name=value. If the "=" is + * missing for some reason, treat it like an empty value. + */ + name = option; + separator = strchr(option, '='); + if (separator) + { + *separator = '\0'; + value = separator + 1; + } + else + value = ""; + + if (i > 0) + appendPQExpBufferStr(buffer, ", "); + appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name)); + + /* + * In general we need to quote the value; but to avoid unnecessary + * clutter, do not quote if it is an identifier that would not need + * quoting. (We could also allow numbers, but that is a bit trickier + * than it looks --- for example, are leading zeroes significant? We + * don't want to assume very much here about what custom reloptions + * might mean.) + */ + if (strcmp(fmtId(value), value) == 0) + appendPQExpBufferStr(buffer, value); + else + appendStringLiteral(buffer, value, encoding, std_strings); + } + + if (options) + free(options); + + return true; +} + + +/* * processSQLNamePattern * * Scan a wildcard-pattern string and generate appropriate WHERE clauses diff --git a/src/include/fe_utils/string_utils.h b/src/include/fe_utils/string_utils.h index 5d3fcc2..733e337 100644 --- a/src/include/fe_utils/string_utils.h +++ b/src/include/fe_utils/string_utils.h @@ -42,6 +42,9 @@ extern void appendByteaLiteral(PQExpBuffer buf, extern bool parsePGArray(const char *atext, char ***itemarray, int *nitems); +extern bool appendReloptionsArray(PQExpBuffer buffer, const char *reloptions, + const char *prefix, int encoding, bool std_strings); + extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, bool have_where, bool force_escape, -- 2.6.6
From 432be5ec0d672840f2b3bd92a1860003bd2658a2 Mon Sep 17 00:00:00 2001 From: Dean Rasheed <dean.a.rash...@gmail.com> Date: Mon, 2 May 2016 12:09:01 +0100 Subject: [PATCH 2/2] Make psql's \ev and \sv commands handle view reloptions. Commit 8eb6407aaeb6cbd972839e356b436bb698f51cff added support for editing and showing view definitions, but neglected to account for view options such as security_barrier and WITH CHECK OPTION which are not returned by pg_get_viewdef() and so need special handling. --- src/bin/psql/command.c | 77 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 4fa7760..87adfce 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -3274,12 +3274,51 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid, * CREATE for ourselves. We must fully qualify the view name to * ensure the right view gets replaced. Also, check relation kind * to be sure it's a view. + * + * Starting with 9.2, views may have reloptions (security_barrier) + * and from 9.4 onwards they may also have WITH [LOCAL|CASCADED] + * CHECK OPTION. These are not part of the view definition + * returned by pg_get_viewdef() and so need to be retrieved + * separately. Materialized views (introduced in 9.3) may have + * arbitrary storage parameter reloptions. */ - printfPQExpBuffer(query, - "SELECT nspname, relname, relkind, pg_catalog.pg_get_viewdef(c.oid, true) FROM " - "pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_namespace n " - "ON c.relnamespace = n.oid WHERE c.oid = %u", - oid); + if (pset.sversion >= 90400) + { + printfPQExpBuffer(query, + "SELECT nspname, relname, relkind, " + "pg_catalog.pg_get_viewdef(c.oid, true), " + "array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, " + "CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text " + "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption " + "FROM pg_catalog.pg_class c " + "LEFT JOIN pg_catalog.pg_namespace n " + "ON c.relnamespace = n.oid WHERE c.oid = %u", + oid); + } + else if (pset.sversion >= 90200) + { + printfPQExpBuffer(query, + "SELECT nspname, relname, relkind, " + "pg_catalog.pg_get_viewdef(c.oid, true), " + "c.reloptions AS reloptions, " + "NULL AS checkoption " + "FROM pg_catalog.pg_class c " + "LEFT JOIN pg_catalog.pg_namespace n " + "ON c.relnamespace = n.oid WHERE c.oid = %u", + oid); + } + else + { + printfPQExpBuffer(query, + "SELECT nspname, relname, relkind, " + "pg_catalog.pg_get_viewdef(c.oid, true), " + "NULL AS reloptions, " + "NULL AS checkoption " + "FROM pg_catalog.pg_class c " + "LEFT JOIN pg_catalog.pg_namespace n " + "ON c.relnamespace = n.oid WHERE c.oid = %u", + oid); + } break; } @@ -3304,6 +3343,8 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid, char *relname = PQgetvalue(res, 0, 1); char *relkind = PQgetvalue(res, 0, 2); char *viewdef = PQgetvalue(res, 0, 3); + char *reloptions = PQgetvalue(res, 0, 4); + char *checkoption = PQgetvalue(res, 0, 5); /* * If the backend ever supports CREATE OR REPLACE @@ -3328,11 +3369,33 @@ get_create_object_cmd(EditableObjectType obj_type, Oid oid, break; } appendPQExpBuffer(buf, "%s.", fmtId(nspname)); - appendPQExpBuffer(buf, "%s AS\n", fmtId(relname)); - appendPQExpBufferStr(buf, viewdef); + appendPQExpBufferStr(buf, fmtId(relname)); + + /* reloptions, if not an empty array "{}" */ + if (reloptions != NULL && strlen(reloptions) > 2) + { + appendPQExpBufferStr(buf, "\n WITH ("); + if (!appendReloptionsArray(buf, reloptions, "", + pset.encoding, + standard_strings())) + { + psql_error("Could not parse reloptions array\n"); + result = false; + } + appendPQExpBufferStr(buf, ")"); + } + + /* View definition from pg_get_viewdef (a SELECT query) */ + appendPQExpBuffer(buf, " AS\n%s", viewdef); + /* Get rid of the semicolon that pg_get_viewdef appends */ if (buf->len > 0 && buf->data[buf->len - 1] == ';') buf->data[--(buf->len)] = '\0'; + + /* WITH [LOCAL|CASCADED] CHECK OPTION */ + if (checkoption && checkoption[0] != '\0') + appendPQExpBuffer(buf, "\n WITH %s CHECK OPTION", + checkoption); } break; } -- 2.6.6
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers