Re: Multiple -t options for pg_dump

On Tue, Sep 20, 2005 at 12:15:23PM -0400, Neil Conway wrote:
> A few minor comments are below. This is for 8.2, right?

I am hoping to make a case for inclusion in 8.1. The code is
completely isolated in one command and does not affect anyone who
does not use the new features.

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

OK, will fix when I get home from work.

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

OK :)

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

OK

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

I tried that. How do I break out of both loops without a goto?

Cheers,
D
--
David Fetter david(at)fetter(dot)org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778

Remember to vote!