On Fri, Aug 9, 2013 at 01:39:35PM -0400, Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > On Fri, Aug 9, 2013 at 12:53:20PM -0400, Tom Lane wrote:
> >> This really requires more than no attention to the comments, especially
> >> since you just removed the only apparent reason for _getObjectDescription
> >> to make a distinction between objects whose name includes a schema and
> >> those that don't.
>
> > I am confused. Are you saying I didn't read the comments, or that I can
> > now merge the schema-qualified and non-schema-qualified object sections?
>
> Well, it's certainly not immediately obvious why we shouldn't merge them.
> But I would have expected the function's header comment to now explain
> that the output is intentionally not schema-qualified and assumes that the
> search path is set for the object's schema if any.
OK, done with the attached patch. The dump output is unchanged.
> > Also, this seems like dead code as there is no test for "INDEX" in the
> > if() block it exists in:
>
> > /*
> > * Pre-7.3 pg_dump would sometimes (not always) put a fmtId'd name
> > * into te->tag for an index. This check is heuristic, so make its
> > * scope as narrow as possible.
> > */
> > if (AH->version < K_VERS_1_7 &&
> > te->tag[0] == '"' &&
> > te->tag[strlen(te->tag) - 1] == '"' &&
> > strcmp(type, "INDEX") == 0)
> > appendPQExpBuffer(buf, "%s", te->tag);
> > else
>
> Huh, yeah it is dead code, since _printTocEntry doesn't call this function
> for "INDEX" objects. And anyway I doubt anybody still cares about reading
> 7.2-era archive files. No objection to removing that.
Removed.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index cd7669b..5204ceb
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** _selectTablespace(ArchiveHandle *AH, con
*** 2879,2889 ****
/*
* Extract an object description for a TOC entry, and append it to buf.
*
! * This is not quite as general as it may seem, since it really only
! * handles constructing the right thing to put into ALTER ... OWNER TO.
! *
! * The whole thing is pretty grotty, but we are kind of stuck since the
! * information used is all that's available in older dump files.
*/
static void
_getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
--- 2879,2885 ----
/*
* Extract an object description for a TOC entry, and append it to buf.
*
! * This is used for ALTER ... OWNER TO.
*/
static void
_getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
*************** _getObjectDescription(PQExpBuffer buf, T
*** 2895,2901 ****
strcmp(type, "MATERIALIZED VIEW") == 0)
type = "TABLE";
! /* objects named by a schema and name */
if (strcmp(type, "COLLATION") == 0 ||
strcmp(type, "CONVERSION") == 0 ||
strcmp(type, "DOMAIN") == 0 ||
--- 2891,2897 ----
strcmp(type, "MATERIALIZED VIEW") == 0)
type = "TABLE";
! /* objects that don't require special decoration */
if (strcmp(type, "COLLATION") == 0 ||
strcmp(type, "CONVERSION") == 0 ||
strcmp(type, "DOMAIN") == 0 ||
*************** _getObjectDescription(PQExpBuffer buf, T
*** 2903,2937 ****
strcmp(type, "TYPE") == 0 ||
strcmp(type, "FOREIGN TABLE") == 0 ||
strcmp(type, "TEXT SEARCH DICTIONARY") == 0 ||
! strcmp(type, "TEXT SEARCH CONFIGURATION") == 0)
! {
! appendPQExpBuffer(buf, "%s ", type);
! if (te->namespace && te->namespace[0]) /* is null pre-7.3 */
! appendPQExpBuffer(buf, "%s.", fmtId(te->namespace));
!
! /*
! * Pre-7.3 pg_dump would sometimes (not always) put a fmtId'd name
! * into te->tag for an index. This check is heuristic, so make its
! * scope as narrow as possible.
! */
! if (AH->version < K_VERS_1_7 &&
! te->tag[0] == '"' &&
! te->tag[strlen(te->tag) - 1] == '"' &&
! strcmp(type, "INDEX") == 0)
! appendPQExpBuffer(buf, "%s", te->tag);
! else
! appendPQExpBuffer(buf, "%s", fmtId(te->tag));
! return;
! }
!
! /* objects named by just a name */
! if (strcmp(type, "DATABASE") == 0 ||
strcmp(type, "PROCEDURAL LANGUAGE") == 0 ||
strcmp(type, "SCHEMA") == 0 ||
strcmp(type, "FOREIGN DATA WRAPPER") == 0 ||
strcmp(type, "SERVER") == 0 ||
strcmp(type, "USER MAPPING") == 0)
{
appendPQExpBuffer(buf, "%s %s", type, fmtId(te->tag));
return;
}
--- 2899,2914 ----
strcmp(type, "TYPE") == 0 ||
strcmp(type, "FOREIGN TABLE") == 0 ||
strcmp(type, "TEXT SEARCH DICTIONARY") == 0 ||
! strcmp(type, "TEXT SEARCH CONFIGURATION") == 0 ||
! /* non-schema-specified objects */
! strcmp(type, "DATABASE") == 0 ||
strcmp(type, "PROCEDURAL LANGUAGE") == 0 ||
strcmp(type, "SCHEMA") == 0 ||
strcmp(type, "FOREIGN DATA WRAPPER") == 0 ||
strcmp(type, "SERVER") == 0 ||
strcmp(type, "USER MAPPING") == 0)
{
+ /* We already know that search_path was set properly */
appendPQExpBuffer(buf, "%s %s", type, fmtId(te->tag));
return;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
