Re: proposal: possibility to read dumped table's name from file

so 27. 6. 2020 v 14:55 odesílatel vignesh C <vignesh21@gmail.com> napsal:

> On Thu, Jun 11, 2020 at 1:07 PM Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
> >
> > Thank you for comments, attached updated patch
> >
>
> Few comments:
> +invalid_filter_format(char *message, char *filename, char *line, int
> lineno)
> +{
> +       char       *displayname;
> +
> +       displayname = *filename == '-' ? "stdin" : filename;
> +
> +       pg_log_error("invalid format of filter file \"%s\": %s",
> +                                displayname,
> +                                message);
> +
> +       fprintf(stderr, "%d: %s\n", lineno, line);
> +       exit_nicely(1);
> +}
>
I think fclose is missing here.
>

done


>
> +                                               if (line[chars - 1] ==
> '\n')
> +                                                       line[chars - 1] =
> '\0';
> Should we check for '\r' also to avoid failures in some platforms.
>

I checked other usage of fgets in Postgres source code, and everywhere is
used test on \n

When I did some fast research, then
https://stackoverflow.com/questions/12769289/carriage-return-by-fgets \r in
this case should be thrown by libc on Microsoft

https://stackoverflow.com/questions/2061334/fgets-linux-vs-mac

\n should be on Mac OS X .. 2001 year .. I am not sure if Mac OS 9 should
be supported.




>
> +     <varlistentry>
> +      <term><option>--filter=<replaceable
> class="parameter">filename</replaceable></option></term>
> +      <listitem>
> +       <para>
> +        Read filters from file. Format "(+|-)(tnfd) objectname:
> +       </para>
> +      </listitem>
> +     </varlistentry>
>
> I felt some documentation is missing here. We could include,
> options tnfd is for controlling table, schema, foreign server data &
> table exclude patterns.
>

I have a plan to completate doc when the design is completed. It was not
clear if people prefer long or short forms of option names.


> Instead of using tnfd, if we could use the same options as existing
> pg_dump options it will be less confusing.
>

it almost same

+-t .. tables
+-n schema
-d exclude data .. there is not short option for --exclude-table-data
+f include foreign table .. there is not short option for
--include-foreign-data

So still, there is a opened question if use +-tnfd system, or system based
on long option

table foo
exclude-table foo
schema xx
exclude-schema xx
include-foreign-data yyy
exclude-table-data zzz


Typically these files will be generated by scripts and processed via pipe,
so there I see just two arguments for and aginst:

short format - there is less probability to do typo error (but there is not
full consistency with pg_dump options)
long format - it is self documented (and there is full consistency with
pg_dump)

In this case I prefer short form .. it is more comfortable for users, and
there are only a few variants, so it is not necessary to use too verbose
language (design). But my opinion is not aggressively strong and I'll
accept any common agreement.

Regards

Updated patch attached



> Regards,
> Vignesh
> EnterpriseDB: http://www.enterprisedb.com
>