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
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 (?) |