Skip site navigation (1) Skip section navigation (2)

Re: Multiple -t options for pg_dump

From: Neil Conway <neilc(at)samurai(dot)com>
To: David Fetter <david(at)fetter(dot)org>
Cc: PostgreSQL Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Multiple -t options for pg_dump
Date: 2005-09-20 16:15:23
Message-ID: 1127232923.17597.11.camel@localhost.localdomain (view raw or flat)
Thread:
Lists: pgsql-patches
A few minor comments are below. This is for 8.2, right?

On Tue, 2005-20-09 at 08:51 -0700, David Fetter wrote:
>   /* obsolete as of 7.3: */
>   static Oid    g_last_builtin_oid; /* value of the last builtin oid
> */
>   
> ! static char **selectTableNames = NULL;        /* name(s) of
> specified table(s) to dump */
> ! int           tab_max = 0, tab_idx = 0;
> ! static char *selectSchemaName = NULL; /* name(s) of specified
> schema(ta) to dump */
> ! int           schem_max = 0, schem_idx = 0;
>   
>   char          g_opaque_type[10];      /* name for the opaque type */

AFAICS the schema changes are unused. However, it would be worth
enhancing -n to allow it to be specified multiple times as well. And to
nit-pick, "tab_max" and "tab_idx" should be static. Also, within the PG
source the plural of "schema" is "schemas".

> !                       case 't':                       /* Dump data
> for th(is|ese) table(s) only */
> !                               if (tab_idx == tab_max) {
> !                                       tab_max += 32;
> !                                       if ( (selectTableNames =
> realloc(selectTableNames, tab_max*sizeof(char *))) == 0)
> !                                               exit(-1);
> !                               }

If you're going to check for calloc failure, you may as well check for
strdup failure as well. Also, exit(1) would be more consistent with the
rest of pg_dump. But personally I wouldn't bother checking for calloc
(or strdup) failure, as the rest of pg_dump doesn't.

> *** 2414,2420 ****
>   {
>         PGresult   *res;
>         int                     ntups;
> !       int                     i;
>         PQExpBuffer query = createPQExpBuffer();
>         PQExpBuffer delqry = createPQExpBuffer();
>         PQExpBuffer lockquery = createPQExpBuffer();
> --- 2427,2433 ----
>   {
>         PGresult   *res;
>         int                     ntups;
> !       int                     i,j;
>         PQExpBuffer query = createPQExpBuffer();
>         PQExpBuffer delqry = createPQExpBuffer();
>         PQExpBuffer lockquery = createPQExpBuffer();

You should move j's definition to the scope closest to its use.

> --- 2706,2724 ----
>          * simplistic since we don't fully check the combination of -n
> and -t
>          * switches.)
>          */
> !       if (selectTableNames != NULL)
>         {
> !               for (i = 0; i < ntups; i++) {
> !                       for (j = 0; j < tab_idx; j++) {
> !                               if (strcmp(tblinfo[i].dobj.name,
> selectTableNames[j]) == 0)
> !                                       goto check_match;
> !                       }
> !               }
>                 /* Didn't find a match */
> ! check_match: if (i == ntups)
>                 {
>                         write_msg(NULL, "specified table \"%s\" does
> not exist\n",
> !                                         selectTableNames[j]);
>                         exit_nicely();
>                 }
>         }

AFAICS the goto is unnecessary -- just break out of the loop when you
find a match, and test i == ntups outside the loop.

-Neil



In response to

Responses

pgsql-patches by date

Next:From: David FetterDate: 2005-09-20 18:33:39
Subject: Re: Multiple -t options for pg_dump
Previous:From: David FetterDate: 2005-09-20 15:51:50
Subject: Multiple -t options for pg_dump

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group