Re: proposal: possibility to read dumped table's name from file - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: proposal: possibility to read dumped table's name from file
Date
Msg-id CAFj8pRB4g-rS+ZhHN8QFbZNpZ3M2MF-2_FxTYLYK=KckzapBeg@mail.gmail.com
Whole thread Raw
In response to Re: proposal: possibility to read dumped table's name from file  (vignesh C <vignesh21@gmail.com>)
List pgsql-hackers


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


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

pgsql-hackers by date:

Previous
From: Paul A Jungwirth
Date:
Subject: Re: range_agg
Next
From: Pavel Stehule
Date:
Subject: Re: proposal: possibility to read dumped table's name from file