From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: proposal: possibility to read dumped table's name from file |
Date: | 2020-06-09 22:30:42 |
Message-ID: | 20200609223042.GF26811@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jun 09, 2020 at 11:46:24AM +0200, Pavel Stehule wrote:
> po 8. 6. 2020 v 23:30 odesílatel Justin Pryzby <pryzby(at)telsasoft(dot)com> napsal:
> I still wonder if a better syntax would use a unified --filter option, whose
> > argument would allow including/excluding any type of object:
> I tried to implement simple format "[+-][tndf] objectname"
Thanks.
> + /* ignore empty rows */
> + if (*line != '\0')
Maybe: if line=='\0': continue
We should also support comments.
> + bool include_filter = false;
> + bool exclude_filter = false;
I think we only need one bool.
You could call it: bool is_exclude = false
> +
> + if (chars < 4)
> + invalid_filter_format(optarg, line, lineno);
I think that check is too lax.
I think it's ok if we require the first char to be [-+] and the 2nd char to be
[dntf]
> + objecttype = line[1];
... but I think this is inadequately "liberal in what it accepts"; I think it
should skip spaces. In my proposed scheme, someone might reasonably write:
> +
> + objectname = &line[3];
> +
> + /* skip initial spaces */
> + while (*objectname == ' ')
> + objectname++;
I suggest to use isspace()
I think we should check that *objectname != '\0', rather than chars>=4, above.
> + if (include_filter)
> + {
> + simple_string_list_append(&table_include_patterns, objectname);
> + dopt.include_everything = false;
> + }
> + else if (exclude_filter)
> + simple_string_list_append(&table_exclude_patterns, objectname);
If you use bool is_exclude, then this becomes "else" and you don't need to
think about checking if (!include && !exclude).
> + else if (objecttype == 'f')
> + {
> + if (include_filter)
> + simple_string_list_append(&foreign_servers_include_patterns, objectname);
> + else if (exclude_filter)
> + invalid_filter_format(optarg, line, lineno);
> + }
I would handle invalid object types as "else: invalid_filter_format()" here,
rather than duplicating above as: !=ALL('d','n','t','f')
> +
> + if (ferror(f))
> + fatal("could not read from file \"%s\": %s",
> + f == stdin ? "stdin" : optarg,
> + strerror(errno));
I think we're allowed to use %m here ?
> + printf(_(" --filter=FILENAME read object names from file\n"));
Object name filter expression, or something..
> + * getline is originaly GNU function, and should not be everywhere still.
originally
> + * Use own reduced implementation.
Did you "reduce" this from another implementation? Where?
What is its license ?
Maybe a line-reader already exists in the frontend (?) .. or maybe it should.
--
Justin
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2020-06-09 22:50:26 | Re: Auto-vectorization speeds up multiplication of large-precision numerics |
Previous Message | Peter Eisentraut | 2020-06-09 22:24:50 | Re: Fixed the remaining old function names. |