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
Views: Raw Message | Whole Thread | Download mbox | Resend email
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

Browse pgsql-patches by date

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