Re: Multiple -t options for pg_dump - Mailing list 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