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
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 |