Horton

Re: Extended Statistics set/restore/clear functions.

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Corey Huinker <corey.huinker@gmail.com>, jian he <jian.universality@gmail.com>, Tomas Vondra <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org
Date: 2025-12-05T00:37:32Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add test doing some cloning of extended statistics data

  2. Add test for pg_restore_extended_stats() with multiranges

  3. Add support for "mcv" in pg_restore_extended_stats()

  4. Include extended statistics data in pg_dump

  5. Add support for "dependencies" in pg_restore_extended_stats()

  6. Add test for MAINTAIN permission with pg_restore_extended_stats()

  7. Add pg_restore_extended_stats()

  8. Add routine to free MCVList

  9. Improve pg_clear_extended_stats() with incorrect relation/stats combination

  10. Add pg_clear_extended_stats()

  11. Introduce routines to validate and free MVNDistinct and MVDependencies

  12. Fix typo in stat_utils.c

  13. Move attribute statistics functions to stat_utils.c

  14. Improve error messages of input functions for pg_dependencies and pg_ndistinct

  15. Improve test output of extended statistics for ndistinct and dependencies

  16. Fix some compiler warnings

  17. Add input function for data type pg_dependencies

  18. Add input function for data type pg_ndistinct

  19. Rework output format of pg_dependencies

  20. Rework output format of pg_ndistinct

  21. Fix comments of output routines for pg_ndistinct and pg_dependencies

  22. Move code specific to pg_dependencies to new file

  23. Move code specific to pg_ndistinct to new file

  24. Document some structures in attribute_stats.c

  25. Fix FATAL message for invalid recovery timeline at beginning of recovery

Michael Paquier <michael@paquier.xyz> 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