Thread: wrong comments in ClassifyUtilityCommandAsReadOnly

hi.

/*
 * Determine the degree to which a utility command is read only.
 *
 * Note the definitions of the relevant flags in src/include/utility/tcop.h.
 */
static int
ClassifyUtilityCommandAsReadOnly(Node *parsetree)

Is the comment wrong?

it should be
" * Note the definitions of the relevant flags in src/include/tcop/utility.h."

David Rowley <dgrowleyml@gmail.com> writes:
> + *      The "DefineAggregate" routine take the parse tree and pick out the
> + *      appropriate arguments/flags, passing the results to
> + *      "AggregateCreate" routine (src src/backend/catalog) that do the actual
> + *      catalog-munging.  These routines also verify permission of the user to
> + *      execute the command.

The grammar needs some help here.  Also, rather than simply remove
define.c's entire header comment, maybe we should write something
relevant about what it does?  Good catches otherwise.

            regards, tom lane

David Rowley <dgrowleyml@gmail.com> writes:
> On Mon, 23 Dec 2024 at 16:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Also, rather than simply remove
>> define.c's entire header comment, maybe we should write something
>> relevant about what it does?  Good catches otherwise.

> I didn't have any inspiration on what to write other than what's
> already written on line 4.

Hmm ... fair enough, I don't have a tighter spec either.  It looks
like the current situation is my fault --- 71dc300a3 should have
thought harder about how to update this header comment.

> Another reason I deleted that is that
> since the file contains helper functions, I didn't want to write a new
> comment based on what functions are there now as it may put someone
> else off from adding new ones if the new one doesn't fit the comment.

Perhaps we could define it as "Support routines for dealing with
DefElem nodes".  You're right that maybe someone would want to
throw in something else, but would it really belong?  The file's
charter seems far narrower now than it once was.

            regards, tom lane

On Mon, 23 Dec 2024 at 18:53, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Rowley <dgrowleyml@gmail.com> writes:
> > Another reason I deleted that is that
> > since the file contains helper functions, I didn't want to write a new
> > comment based on what functions are there now as it may put someone
> > else off from adding new ones if the new one doesn't fit the comment.
>
> Perhaps we could define it as "Support routines for dealing with
> DefElem nodes".  You're right that maybe someone would want to
> throw in something else, but would it really belong?  The file's
> charter seems far narrower now than it once was.

I felt that it was better to leave the scope a bit wider than that,
but I don't feel very strongly, so I've pushed it with your wording
suggestion.

Thanks

David