wrong comments in ClassifyUtilityCommandAsReadOnly
Started by jian heabout 1 year ago6 messages
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."
Re: wrong comments in ClassifyUtilityCommandAsReadOnly
On Sat, 21 Dec 2024 at 17:06, jian he <jian.universality@gmail.com> wrote:
* 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."
Yeah. There is no tcop.h.
I used the attached (not exactly very pretty) script to find a few more.
git grep -h -E "\Wsrc/[\w_/]*" -- '*.[ch]' | tr -d '"' | tr -d \'\" |
xargs -n 1 ./fileexists.sh | grep "does not"
There are plenty of places that reference files not starting with
"src/". It might be good to verify there's some subdirectory in the
source tree that match those, however, I didn't think of a creative
way to identify those ones.
David
Attachments:
fix_incorrect_filename_references.patchapplication/octet-stream; name=fix_incorrect_filename_references.patchDownload
diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c
index fde5a2a0e0..615ac5817c 100644
--- a/src/backend/commands/aggregatecmds.c
+++ b/src/backend/commands/aggregatecmds.c
@@ -12,11 +12,11 @@
* src/backend/commands/aggregatecmds.c
*
* DESCRIPTION
- * The "DefineFoo" routines take the parse tree and pick out the
- * appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
- * the actual catalog-munging. These routines also verify permission
- * of the user to execute the command.
+ * 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.
*
*-------------------------------------------------------------------------
*/
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 43f50066ea..ed67d3add0 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -11,23 +11,6 @@
* IDENTIFICATION
* src/backend/commands/define.c
*
- * DESCRIPTION
- * The "DefineFoo" routines take the parse tree and pick out the
- * appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
- * the actual catalog-munging. These routines also verify permission
- * of the user to execute the command.
- *
- * NOTES
- * These things must be defined and committed in the following order:
- * "create function":
- * input/output, recv/send procedures
- * "create type":
- * type
- * "create operator":
- * operators
- *
- *
*-------------------------------------------------------------------------
*/
#include "postgres.h"
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 6fc3863265..82dd659893 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -15,7 +15,7 @@
* DESCRIPTION
* These routines take the parse tree and pick out the
* appropriate arguments/flags, and pass the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index 5872a3e192..ec8b053ba4 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -14,7 +14,7 @@
* DESCRIPTION
* The "DefineFoo" routines take the parse tree and pick out the
* appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 4f20b5be06..a6bb0b70f5 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -13,7 +13,7 @@
* DESCRIPTION
* The "DefineFoo" routines take the parse tree and pick out the
* appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 33dea5a781..c2ed8214ef 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -122,7 +122,7 @@ CommandIsReadOnly(PlannedStmt *pstmt)
/*
* Determine the degree to which a utility command is read only.
*
- * Note the definitions of the relevant flags in src/include/utility/tcop.h.
+ * Note the definitions of the relevant flags in src/include/tcop/utility.h.
*/
static int
ClassifyUtilityCommandAsReadOnly(Node *parsetree)
diff --git a/src/backend/utils/misc/guc_internal.h b/src/backend/utils/misc/guc_internal.h
index 9695dcd5b7..91acc1b320 100644
--- a/src/backend/utils/misc/guc_internal.h
+++ b/src/backend/utils/misc/guc_internal.h
@@ -6,7 +6,7 @@
*
* Copyright (c) 2000-2024, PostgreSQL Global Development Group
*
- * src/include/utils/guc_internal.h
+ * src/backend/utils/misc/guc_internal.h
*--------------------------------------------------------------------
*/
#ifndef GUC_INTERNAL_H
diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index 47568da2db..b20ef5019c 100644
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_file.c
+ * src/fe_utils/astreamer_file.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_gzip.c b/src/fe_utils/astreamer_gzip.c
index e0b755317c..8f966a34b5 100644
--- a/src/fe_utils/astreamer_gzip.c
+++ b/src/fe_utils/astreamer_gzip.c
@@ -20,7 +20,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_gzip.c
+ * src/fe_utils/astreamer_gzip.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_lz4.c b/src/fe_utils/astreamer_lz4.c
index a628088edf..98c83139ef 100644
--- a/src/fe_utils/astreamer_lz4.c
+++ b/src/fe_utils/astreamer_lz4.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_lz4.c
+ * src/fe_utils/astreamer_lz4.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_tar.c b/src/fe_utils/astreamer_tar.c
index f5d3562d28..53a956a71d 100644
--- a/src/fe_utils/astreamer_tar.c
+++ b/src/fe_utils/astreamer_tar.c
@@ -15,7 +15,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_tar.c
+ * src/fe_utils/astreamer_tar.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_zstd.c b/src/fe_utils/astreamer_zstd.c
index 4b2d42b231..525ca64a2e 100644
--- a/src/fe_utils/astreamer_zstd.c
+++ b/src/fe_utils/astreamer_zstd.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_zstd.c
+ * src/fe_utils/astreamer_zstd.c
*-------------------------------------------------------------------------
*/
diff --git a/src/include/fe_utils/astreamer.h b/src/include/fe_utils/astreamer.h
index 570cfba304..7aa10b4f5e 100644
--- a/src/include/fe_utils/astreamer.h
+++ b/src/include/fe_utils/astreamer.h
@@ -24,7 +24,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer.h
+ * src/include/fe_utils/astreamer.h
*-------------------------------------------------------------------------
*/
Re: wrong comments in ClassifyUtilityCommandAsReadOnly
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
Re: wrong comments in ClassifyUtilityCommandAsReadOnly
On Mon, 23 Dec 2024 at 16:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.
Ah oops. I forgot to check that before posting.
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. 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.
I'm happy to take suggestions if you can think of something. In the
meantime, I've attached the patch with the aggregatecmds.c comment
fixes.
David
Attachments:
fix_incorrect_filename_references_v2.patchapplication/octet-stream; name=fix_incorrect_filename_references_v2.patchDownload
diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c
index fde5a2a0e0..3f117d8e2c 100644
--- a/src/backend/commands/aggregatecmds.c
+++ b/src/backend/commands/aggregatecmds.c
@@ -12,11 +12,11 @@
* src/backend/commands/aggregatecmds.c
*
* DESCRIPTION
- * The "DefineFoo" routines take the parse tree and pick out the
+ * The "DefineAggregate" routine takes the parse tree and picks out the
* appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
- * the actual catalog-munging. These routines also verify permission
- * of the user to execute the command.
+ * "AggregateCreate" routine (in src/backend/catalog), which does the
+ * actual catalog-munging. DefineAggregate also verifies the permission of
+ * the user to execute the command.
*
*-------------------------------------------------------------------------
*/
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 43f50066ea..ed67d3add0 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -11,23 +11,6 @@
* IDENTIFICATION
* src/backend/commands/define.c
*
- * DESCRIPTION
- * The "DefineFoo" routines take the parse tree and pick out the
- * appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
- * the actual catalog-munging. These routines also verify permission
- * of the user to execute the command.
- *
- * NOTES
- * These things must be defined and committed in the following order:
- * "create function":
- * input/output, recv/send procedures
- * "create type":
- * type
- * "create operator":
- * operators
- *
- *
*-------------------------------------------------------------------------
*/
#include "postgres.h"
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 6fc3863265..82dd659893 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -15,7 +15,7 @@
* DESCRIPTION
* These routines take the parse tree and pick out the
* appropriate arguments/flags, and pass the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index 5872a3e192..ec8b053ba4 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -14,7 +14,7 @@
* DESCRIPTION
* The "DefineFoo" routines take the parse tree and pick out the
* appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 4f20b5be06..a6bb0b70f5 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -13,7 +13,7 @@
* DESCRIPTION
* The "DefineFoo" routines take the parse tree and pick out the
* appropriate arguments/flags, passing the results to the
- * corresponding "FooDefine" routines (in src/catalog) that do
+ * corresponding "FooCreate" routines (in src/backend/catalog) that do
* the actual catalog-munging. These routines also verify permission
* of the user to execute the command.
*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 33dea5a781..c2ed8214ef 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -122,7 +122,7 @@ CommandIsReadOnly(PlannedStmt *pstmt)
/*
* Determine the degree to which a utility command is read only.
*
- * Note the definitions of the relevant flags in src/include/utility/tcop.h.
+ * Note the definitions of the relevant flags in src/include/tcop/utility.h.
*/
static int
ClassifyUtilityCommandAsReadOnly(Node *parsetree)
diff --git a/src/backend/utils/misc/guc_internal.h b/src/backend/utils/misc/guc_internal.h
index 9695dcd5b7..91acc1b320 100644
--- a/src/backend/utils/misc/guc_internal.h
+++ b/src/backend/utils/misc/guc_internal.h
@@ -6,7 +6,7 @@
*
* Copyright (c) 2000-2024, PostgreSQL Global Development Group
*
- * src/include/utils/guc_internal.h
+ * src/backend/utils/misc/guc_internal.h
*--------------------------------------------------------------------
*/
#ifndef GUC_INTERNAL_H
diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index 47568da2db..b20ef5019c 100644
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_file.c
+ * src/fe_utils/astreamer_file.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_gzip.c b/src/fe_utils/astreamer_gzip.c
index e0b755317c..8f966a34b5 100644
--- a/src/fe_utils/astreamer_gzip.c
+++ b/src/fe_utils/astreamer_gzip.c
@@ -20,7 +20,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_gzip.c
+ * src/fe_utils/astreamer_gzip.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_lz4.c b/src/fe_utils/astreamer_lz4.c
index a628088edf..98c83139ef 100644
--- a/src/fe_utils/astreamer_lz4.c
+++ b/src/fe_utils/astreamer_lz4.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_lz4.c
+ * src/fe_utils/astreamer_lz4.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_tar.c b/src/fe_utils/astreamer_tar.c
index f5d3562d28..53a956a71d 100644
--- a/src/fe_utils/astreamer_tar.c
+++ b/src/fe_utils/astreamer_tar.c
@@ -15,7 +15,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_tar.c
+ * src/fe_utils/astreamer_tar.c
*-------------------------------------------------------------------------
*/
diff --git a/src/fe_utils/astreamer_zstd.c b/src/fe_utils/astreamer_zstd.c
index 4b2d42b231..525ca64a2e 100644
--- a/src/fe_utils/astreamer_zstd.c
+++ b/src/fe_utils/astreamer_zstd.c
@@ -9,7 +9,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer_zstd.c
+ * src/fe_utils/astreamer_zstd.c
*-------------------------------------------------------------------------
*/
diff --git a/src/include/fe_utils/astreamer.h b/src/include/fe_utils/astreamer.h
index 570cfba304..7aa10b4f5e 100644
--- a/src/include/fe_utils/astreamer.h
+++ b/src/include/fe_utils/astreamer.h
@@ -24,7 +24,7 @@
* Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/bin/pg_basebackup/astreamer.h
+ * src/include/fe_utils/astreamer.h
*-------------------------------------------------------------------------
*/
Re: wrong comments in ClassifyUtilityCommandAsReadOnly
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
Re: wrong comments in ClassifyUtilityCommandAsReadOnly
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