On Fri, 16 Sep 2022 at 11:51, John Naylor <john.nay...@enterprisedb.com> wrote:
> On Fri, Sep 16, 2022 at 10:11 AM Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote:
>>
>> At Thu, 15 Sep 2022 22:38:01 +0800, Japin Li <japi...@hotmail.com> wrote in
>> >
>> > Hi hacker,
>> >
>> > Recently, I find there might be a typo in xact.c comments. The comments
>> > say "PG_PROC", however, it actually means "PGPROC" structure. Since we
>> > have pg_proc catalog, and use PG_PROC to reference the catalog [1], so,
>> > we should use PGPROC to reference the structure. Any thoughts?
>> >
>> > [1] src/include/nodes/primnodes.h
>>
>> The patch seems to me covering all occurances of PG_PROC as PGPROC.
>
> +1 since this hinders grep-ability.
>
>> I found several uses of PG_PROC as (pg_catalog.)pg_proc, which is
>> quite confusing, too..
>
> It's pretty obvious to me what that refers to in primnodes.h, although
> the capitalization of (some, but not all) catalog names in comments in
> that file is a bit strange. Maybe not worth changing there.
Thanks for the review. I found for system catalog, we often use lower case.
Here attached a new patch to fix it.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
>From 1b1fd4742efe052dbca46006e72411b33a8766cd Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Fri, 16 Sep 2022 11:50:38 +0800
Subject: [PATCH v1 1/2] Use lower case to reference the system catalog
---
src/include/nodes/primnodes.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 40661334bb..48827d291a 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -595,8 +595,8 @@ typedef enum CoercionForm
typedef struct FuncExpr
{
Expr xpr;
- Oid funcid; /* PG_PROC OID of the function */
- Oid funcresulttype; /* PG_TYPE OID of result value */
+ Oid funcid; /* pg_proc OID of the function */
+ Oid funcresulttype; /* pg_type OID of result value */
bool funcretset; /* true if function returns set */
bool funcvariadic; /* true if variadic arguments have been
* combined into an array last argument */
@@ -644,13 +644,13 @@ typedef struct OpExpr
{
Expr xpr;
- /* PG_OPERATOR OID of the operator */
+ /* pg_operator OID of the operator */
Oid opno;
- /* PG_PROC OID of underlying function */
+ /* pg_proc OID of underlying function */
Oid opfuncid pg_node_attr(equal_ignore_if_zero);
- /* PG_TYPE OID of result value */
+ /* pg_type OID of result value */
Oid opresulttype;
/* true if operator returns set */
@@ -721,16 +721,16 @@ typedef struct ScalarArrayOpExpr
{
Expr xpr;
- /* PG_OPERATOR OID of the operator */
+ /* pg_operator OID of the operator */
Oid opno;
- /* PG_PROC OID of comparison function */
+ /* pg_proc OID of comparison function */
Oid opfuncid pg_node_attr(equal_ignore_if_zero);
- /* PG_PROC OID of hash func or InvalidOid */
+ /* pg_proc OID of hash func or InvalidOid */
Oid hashfuncid pg_node_attr(equal_ignore_if_zero);
- /* PG_PROC OID of negator of opfuncid function or InvalidOid. See above */
+ /* pg_proc OID of negator of opfuncid function or InvalidOid. See above */
Oid negfuncid pg_node_attr(equal_ignore_if_zero);
/* true for ANY, false for ALL */
--
2.25.1
>From 4b9ec1f791cbe865866899e8864b6e96d8fa015c Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Fri, 16 Sep 2022 11:52:22 +0800
Subject: [PATCH v1 2/2] Fix a typo about PGPROC structure reference
---
src/backend/access/transam/README | 2 +-
src/backend/access/transam/xact.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README
index 734c39a4d0..72af656060 100644
--- a/src/backend/access/transam/README
+++ b/src/backend/access/transam/README
@@ -198,7 +198,7 @@ parent. This maintains the invariant that child transactions have XIDs later
than their parents, which is assumed in a number of places.
The subsidiary actions of obtaining a lock on the XID and entering it into
-pg_subtrans and PG_PROC are done at the time it is assigned.
+pg_subtrans and PGPROC are done at the time it is assigned.
A transaction that has no XID still needs to be identified for various
purposes, notably holding locks. For this purpose we assign a "virtual
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 50f092d7eb..7abc6a0705 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -680,12 +680,12 @@ AssignTransactionId(TransactionState s)
log_unknown_top = true;
/*
- * Generate a new FullTransactionId and record its xid in PG_PROC and
+ * Generate a new FullTransactionId and record its xid in PGPROC and
* pg_subtrans.
*
* NB: we must make the subtrans entry BEFORE the Xid appears anywhere in
- * shared storage other than PG_PROC; because if there's no room for it in
- * PG_PROC, the subtrans entry is needed to ensure that other backends see
+ * shared storage other than PGPROC; because if there's no room for it in
+ * PGPROC, the subtrans entry is needed to ensure that other backends see
* the Xid as "running". See GetNewTransactionId.
*/
s->fullTransactionId = GetNewTransactionId(isSubXact);
--
2.25.1
