Re: Wrong comment in statscmds.c/CreateStatistics?

From: Junwang Zhao <zhjwpku(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Wrong comment in statscmds.c/CreateStatistics?
Date: 2022-08-16 02:27:38
Message-ID: CAEG8a3LfyTy9yudgZnOzVAhG+LyjvcsVgMjtzJqQrG6HfwUtKQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Yeah, the comments are kind of confusing, see some comments inline.

On Tue, Aug 16, 2022 at 8:47 AM Peter Smith <smithpb2250(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2022-08-16 02:53:18 Re: SELECT documentation
Previous Message Bharath Rupireddy 2022-08-16 02:25:27 Add find_in_log() and advance_wal() perl functions to core test framework (?)