Horton

Re: Extended Statistics set/restore/clear functions.

Yuefei Shi <shiyuefei1004@gmail.com>

View thread
From: Yuefei Shi <shiyuefei1004@gmail.com>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Michael Paquier <michael@paquier.xyz>, Tomas Vondra <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Date: 2025-11-21T05:33:56Z
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

On Fri, Nov 21, 2025 at 10:54 AM Corey Huinker <corey.huinker@gmail.com>
wrote:

>
>> some of the switch->default, default don't have ``break``.
>>
>
> The compiler doesn't require them, but I see that we do use them in a lot
> of places, so I'll incorporate this.
>
>
>>
>> + for (int i = 0; i < nitems; i++)
>> + {
>> + MVNDistinctItem *item =
>> parse_state.distinct_items->elements[i].ptr_value;
>>
>> exposing the ptr_value seems not a good idea, we can foreach_ptr
>> the attached patch using foreach_ptr.
>
>
> I didn't like this because it makes getting the index value harder, and
> the index value is needed in lots of places. Instead I used list_nth() and
> list_nth_int().
>
>
>
>> in function pg_ndistinct_in some errsave can change to ereturn.
>> (I didn't do this part, though).
>>
>
> -0.25
>
> There's something that I like about the consistency of errsave() followed
> by a return that makes it clear that "the function ends here" that I don't
> get from ereturn().
>
> What I would really like, is a way to generate unique (but translated)
> errdetail values, and move the errsave/ereturn to after the switch
> statement.
>
>
>>
>> + /*
>> + * The attnum cannot be zero a negative number beyond the number of the
>> + * possible expressions.
>> + */
>> + if (attnum == 0 || attnum < (0-STATS_MAX_DIMENSIONS))
>> + {
>> + errsave(parse->escontext,
>> + errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
>> + errmsg("malformed pg_ndistinct: \"%s\"", parse->str),
>> + errdetail("Invalid \"%s\" element: %d.",
>> + PG_NDISTINCT_KEY_ATTRIBUTES, attnum));
>> + return JSON_SEM_ACTION_FAILED;
>> + }
>> This part had no coverage tests, so I added a few.
>>
>
> +1
>
>
>
>>
>>
>> as mentioned before
>> + errsave(parse->escontext,
>> + errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
>> + errmsg("malformed pg_ndistinct: \"%s\"", parse->str),
>> + errdetail("The \"%s\" key must contain an array of at least %d "
>> +  "and no more than %d attributes.",
>> +  PG_NDISTINCT_KEY_NDISTINCT, 2, STATS_MAX_DIMENSIONS));
>> here PG_NDISTINCT_KEY_NDISTINCT, should be PG_NDISTINCT_KEY_ATTRIBUTES.
>>
>
> +1
>
> Did similar things to pg_dependencies.
>
>
A few small comments.

1. Minor typo fixes:
In pg_dependencies_in: "pg_dependencies parssing claims..." could be
corrected to "pg_dependencies parsing claims..."
In ndistinct_array_end: "must be an non-empty array" might be better as
"must be a non-empty array"
In the comment for dependencies_object_field_start: "depeendency" appears
to be a typo for "dependency"

2.Code maintainability suggestion:
I noticed the string "malformed pg_dependencies: "%s"" is used repeatedly
throughout the code. Would you consider defining this as a macro? This
could reduce duplication and make future updates easier.

3.Memory management observation:
Regarding item_attnum_list, while PostgreSQL's memory context mechanism
handles cleanup, explicitly freeing the allocated memory after use might
improve code clarity.

These are all minor points - the implementation looks solid overall. Thank
you for your work on this feature!