Re: Extended Statistics set/restore/clear functions.

>
>
> 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.