Horton

Thread

Commits

  1. Quote filenames in error messages

  1. Quoting filename in using facing log messages

    Daniel Gustafsson <daniel@yesql.se> — 2023-09-13T11:48:12Z

    Looking at ZQFUGOuS5DU4DsE4@paquier.xyz I noticed that we had a a few instances
of filenames in userfacing log messages (ie not elog or DEBUGx etc) not being
quoted, where the vast majority are quoted like \"%s\".  Any reason not to
quote them as per the attached to be consistent across all log messages?

--
Daniel Gustafsson

  2. Re: Quoting filename in using facing log messages

    Peter Eisentraut <peter@eisentraut.org> — 2023-09-13T11:55:57Z

    On 13.09.23 13:48, Daniel Gustafsson wrote:
> Looking at ZQFUGOuS5DU4DsE4@paquier.xyz I noticed that we had a a few instances
> of filenames in userfacing log messages (ie not elog or DEBUGx etc) not being
> quoted, where the vast majority are quoted like \"%s\".  Any reason not to
> quote them as per the attached to be consistent across all log messages?

Since WAL file names have a predictable format, there is less pressure 
to quote them to avoid ambiguities.  But in general we should try to be 
consistent, so your patch makes sense to me.

  3. Re: Quoting filename in using facing log messages

    Daniel Gustafsson <daniel@yesql.se> — 2023-09-13T12:02:47Z

    > On 13 Sep 2023, at 13:55, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 13.09.23 13:48, Daniel Gustafsson wrote:
>> Looking at ZQFUGOuS5DU4DsE4@paquier.xyz I noticed that we had a a few instances
>> of filenames in userfacing log messages (ie not elog or DEBUGx etc) not being
>> quoted, where the vast majority are quoted like \"%s\".  Any reason not to
>> quote them as per the attached to be consistent across all log messages?
> 
> Since WAL file names have a predictable format, there is less pressure to quote them to avoid ambiguities.  But in general we should try to be consistent

Correct, this is all for consistency.

> so your patch makes sense to me.

Thanks!

It might be worth concatenating the errmsg() while there since we typically
don't linebreak errmsg strings anymore for greppability:

-	 errmsg("could not write to log file %s "
-		"at offset %u, length %zu: %m",
+	 errmsg("could not write to log file \"%s\" at offset %u, length %zu: %m",

I don't have strong feelings wrt that, just have a vague memory of "concatenate
when touching" as an informal guideline.

--
Daniel Gustafsson

  4. Re: Quoting filename in using facing log messages

    Michael Paquier <michael@paquier.xyz> — 2023-09-14T07:56:03Z

    On Wed, Sep 13, 2023 at 02:02:47PM +0200, Daniel Gustafsson wrote:
> It might be worth concatenating the errmsg() while there since we typically
> don't linebreak errmsg strings anymore for greppability:
> 
> -	 errmsg("could not write to log file %s "
> -		"at offset %u, length %zu: %m",
> +	 errmsg("could not write to log file \"%s\" at offset %u, length %zu: %m",
> 
> I don't have strong feelings wrt that, just have a vague memory of "concatenate
> when touching" as an informal guideline.

Because these are slightly easier to grep when looking for a given
pattern in the tree.

(I'm OK with your patch as well, FWIW.)
--
Michael

  5. Re: Quoting filename in using facing log messages

    Daniel Gustafsson <daniel@yesql.se> — 2023-09-14T09:23:09Z

    > On 14 Sep 2023, at 09:56, Michael Paquier <michael@paquier.xyz> wrote:

> (I'm OK with your patch as well, FWIW.)

Thanks for looking, pushed.

--
Daniel Gustafsson