Yeah, the comments are kind of confusing, see some comments inline.
On Tue, Aug 16, 2022 at 8:47 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> I happened to notice the following code in
> src/backend/commands/statscmds.c, CreateStatistics:
>
> ======
> /*
> * Parse the statistics kinds.
> *
> * First check that if this is the case with a single expression, there
> * are no statistics kinds specified (we don't allow that for the simple
maybe change to *there should be no* is better?
> * CREATE STATISTICS form).
> */
> if ((list_length(stmt->exprs) == 1) && (list_length(stxexprs) == 1))
> {
> /* statistics kinds not specified */
remove this line or change to *statistics kinds should not be specified*,
I prefer just removing it.
> if (list_length(stmt->stat_types) > 0)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("when building statistics on a single expression, statistics
> kinds may not be specified")));
change *may* to *should*?
> }
> ======
>
>
> AFAICT that one-line comment (/* statistics kinds not specified */) is
> wrong because at that point we don't yet know if kinds are specified
> or not.
>
> SUGGESTION-1
> Change the comment to /* Check there are no statistics kinds specified */
>
> SUGGESTION-2
> Simply remove that one-line comment because the larger comment seems
> to be saying the same thing anyhow.
>
> Thoughts?
>
> ------
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
>
>
--
Regards
Junwang Zhao
