Date: Sat, 11 Jul 2020 18:06:50 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: proposal: possibility to read dumped table's name from file
Message-ID: <20200711230650.GY26220@telsasoft.com>
References: <20200529182514.GL17850@telsasoft.com>
 <CAFj8pRDGTeb9OtXqFiE-tnYHCV=HyZQ2T_7S5v_50eJqAnLDwg@mail.gmail.com>
 <20200608212957.GB22473@telsasoft.com>
 <CAFj8pRBP105Qa=2q9MEy_PWmCZGaWd4nt4nB2Gv8WgBO7UbkwA@mail.gmail.com>
 <20200609223042.GF26811@telsasoft.com>
 <CAFj8pRDk6EQGpbVO5tOsDvNnw41wmYWS7-w5Ym8gyRjvfoZ64g@mail.gmail.com>
 <20200701212452.GF4107@telsasoft.com>
 <20200705203119.GT4107@telsasoft.com>
 <CAFj8pRCCJ1vqDREwK41tbRX8k96VSAWFe9uOCCAmLJ8cwJC4Vw@mail.gmail.com>
 <CAFj8pRCRQOWLm2LrWcbkcm1Dx7yTuLks1_2VrTJO3PARDNXO3w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFj8pRCRQOWLm2LrWcbkcm1Dx7yTuLks1_2VrTJO3PARDNXO3w@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On Mon, Jul 06, 2020 at 06:34:15AM +0200, Pavel Stehule wrote:
> >> > Comment support is still missing but easily added :)
> >> Still missing from the latest patch.
> >
> > I can implement a comment support. But I am not sure about the format. The
> 
> here is support for comment's line - first char should be #

Thanks, that's how I assumed it would look.

> >> With some added documentation, I think this can be RfC.

Do you want to add any more documentation ?  

Few more things:

> +exit_invalid_filter_format(FILE *fp, char *filename, char *message, char *line, int lineno)
> +{
> +	pg_log_error("invalid format of filter file \"%s\": %s",
> +				 *filename == '-' ? "stdin" : filename,
> +				 message);

You refer to as "stdin" any filename beginning with -.

I think you could just print "-" and not "stdin".
In any case, *filename=='-' is wrong since it only checks filename[0].
In a few places you compare ==stdin (which is right).

Also, I think "f" isn't as good a name as "fp".

You're adding 139 lines to a 600 line main(), and no other option is more than
15 lines.  Would you put it in a separate function ?

-- 
Justin