public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tom Lane <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Corey Huinker <[email protected]>
Cc: jian he <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: [email protected]
Subject: Re: Extended Statistics set/restore/clear functions.
Date: Thu, 04 Dec 2025 19:37:32 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CACJufxGGzkb58BU+YyTa9cBAawhybwk2cPFZ1XupS-8xuAzN9A@mail.gmail.com>
	<[email protected]>
	<CADkLM=czQ0UUyBn3AiHZfJJWRwXH5e2xn056yG0mpuXpW09RSw@mail.gmail.com>
	<CACJufxHVKiVEvCsc11oxOM=0GgjqChFwi-ii4RJuw95ZFB_fHQ@mail.gmail.com>
	<CADkLM=eRop0t==hg0TU3KuDH6DgnUxQ6vZxV9th=6vW1JG36MQ@mail.gmail.com>
	<CACJufxGgEnOzFq0HjmkKBTnEUgToGFC0g0A4wPWoJMzOuoWUhw@mail.gmail.com>
	<CADkLM=crdyK3w7-EDEdDfXHGidxm2ms+bQYNFOZUD4eOLuLXEQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>

Michael Paquier <[email protected]> writes:
> Okay.  I have gone other these two and after an extra round of
> polishing, mainly around comments (some suggested by Chao, actually),
> style, a fix for the test with valid UTF8 sequences failing in
> non-UTF8 databases, plus a few more tests that were missing, I have
> applied the two patches for the input functions.

I notice that since e1405aa5e went in, a number of older buildfarm
animals are issuing warnings about it:

In file included from pg_dependencies.c:21:0:
pg_dependencies.c: In function "dependencies_scalar":
../../../../src/include/nodes/miscnodes.h:54:15: warning: the comparison will always evaluate as "true" for the address of "escontext" will never be NULL [-Waddress]
  ((escontext) != NULL && IsA(escontext, ErrorSaveContext) && \\
               ^
pg_dependencies.c:497:8: note: in expansion of macro "SOFT_ERROR_OCCURRED"
    if (SOFT_ERROR_OCCURRED(&escontext))
        ^
../../../../src/include/nodes/miscnodes.h:54:15: warning: the comparison will always evaluate as "true" for the address of "escontext" will never be NULL [-Waddress]
  ((escontext) != NULL && IsA(escontext, ErrorSaveContext) && \\
               ^
pg_dependencies.c:542:8: note: in expansion of macro "SOFT_ERROR_OCCURRED"
    if (SOFT_ERROR_OCCURRED(&escontext))
        ^
../../../../src/include/nodes/miscnodes.h:54:15: warning: the comparison will always evaluate as "true" for the address of "escontext" will never be NULL [-Waddress]
  ((escontext) != NULL && IsA(escontext, ErrorSaveContext) && \\
               ^
pg_dependencies.c:572:8: note: in expansion of macro "SOFT_ERROR_OCCURRED"
    if (SOFT_ERROR_OCCURRED(&escontext))
        ^
In file included from pg_ndistinct.c:21:0:
pg_ndistinct.c: In function "ndistinct_scalar":
../../../../src/include/nodes/miscnodes.h:54:15: warning: the comparison will always evaluate as "true" for the address of "escontext" will never be NULL [-Waddress]
  ((escontext) != NULL && IsA(escontext, ErrorSaveContext) && \\
               ^
pg_ndistinct.c:438:8: note: in expansion of macro "SOFT_ERROR_OCCURRED"
    if (SOFT_ERROR_OCCURRED(&escontext))
        ^
../../../../src/include/nodes/miscnodes.h:54:15: warning: the comparison will always evaluate as "true" for the address of "escontext" will never be NULL [-Waddress]
  ((escontext) != NULL && IsA(escontext, ErrorSaveContext) && \\
               ^
pg_ndistinct.c:488:9: note: in expansion of macro "SOFT_ERROR_OCCURRED"
    if (!SOFT_ERROR_OCCURRED(&escontext))
         ^

The above is from rhinoceros, but about half a dozen other animals
are reporting the same.  They are all running very old gcc versions,
mostly from RHEL7 or CentOS 7.

This of course arises because the source code is passing the address
of a local ErrorSaveContext variable to that macro.  We don't have
any other instances of that coding pattern AFAICS, so I wonder if
that was really the most adapted way to do it.  Looking at the
code, it seems to be turning around and stuffing a different error
message into a passed-in ErrorSaveContext, which feels a little
weird.

It may be that there's not a better way, in which case I'll probably
just teach my buildfarm-warning-scraper script to ignore these
warnings, as it's already doing for some other warnings from these
same animals (-Wmissing-braces mostly).  But I thought I'd raise a
question about it.

Also, I don't think these errdetail messages meet our style
guidelines:

                        errdetail("Invalid \"%s\" value.", PG_DEPENDENCIES_KEY_ATTRIBUTES));

They're supposed to be complete sentences.

			regards, tom lane





view thread (91+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Extended Statistics set/restore/clear functions.
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox