Re: patch: option --if-exists for pg_dump

Поиск
Список
Период
Сортировка
От Alvaro Herrera
Тема Re: patch: option --if-exists for pg_dump
Дата
Msg-id 20140217171016.GQ6342@eldon.alvh.no-ip.org
обсуждение исходный текст
Ответ на Re: patch: option --if-exists for pg_dump  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
Ответы Re: patch: option --if-exists for pg_dump
Список pgsql-hackers
Jeevan Chalke escribió:

I don't understand this code.  (Well, it's pg_dump.)  Or maybe I do
understand it, and it's not doing what you think it's doing.  I mean, in
this part:

> diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
> index 7fc0288..c08a0d3 100644
> --- a/src/bin/pg_dump/pg_backup_archiver.c
> +++ b/src/bin/pg_dump/pg_backup_archiver.c
> @@ -413,8 +413,84 @@ RestoreArchive(Archive *AHX)
>                  /* Select owner and schema as necessary */
>                  _becomeOwner(AH, te);
>                  _selectOutputSchema(AH, te->namespace);
> -                /* Drop it */
> -                ahprintf(AH, "%s", te->dropStmt);
> +
> +                if (*te->dropStmt != '\0')
> +                {
> +                    /* Inject IF EXISTS clause to DROP part when required. */
> +                    if (ropt->if_exists)

It does *not* modify te->dropStmt, it only sends ahprint() a different
version of what was stored (injected the wanted IF EXISTS clause).  If
that is correct, then why are we, in this other part, trying to remove
the IF EXISTS clause?

> @@ -2942,9 +3018,39 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
>          strcmp(type, "OPERATOR CLASS") == 0 ||
>          strcmp(type, "OPERATOR FAMILY") == 0)
>      {
> -        /* Chop "DROP " off the front and make a modifiable copy */
> -        char       *first = pg_strdup(te->dropStmt + 5);
> -        char       *last;
> +        char        *first;
> +        char        *last;
> +
> +        /*
> +         * Object description is based on dropStmt statement which may have
> +         * IF EXISTS clause.  Thus we need to update an offset such that it
> +         * won't be included in the object description.
> +         */

Maybe I am mistaken and the te->dropStmt already contains the IF EXISTS
bit for some reason; but if so I don't know why that is.  Care to
explain?

I also think that _getObjectDescription() becomes overworked after this
patch.  I wonder if we should be storing te->objIdentity so that we can
construct the ALTER OWNER command without going to as much trouble as
parsing the DROP command.  Is there a way to do that? Maybe we can ask
the server for the object identity, for example.  There is a new
function to do that in 9.3 which perhaps we can now use.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Bruce Momjian
Дата:
Сообщение: Re: Ctrl+C from sh can shut down daemonized PostgreSQL cluster
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: [bug fix] "pg_ctl stop" times out when it should respond quickly