Re: Fix a comment error in logicalrep_write_typ()
On Thu, Jul 11, 2024 at 4:35 PM cca5507 wrote: > > Thank you for review! > > The commitfest link for tracking: > https://commitfest.postgresql.org/49/5121/ > I've pushed and closed the CF entry. -- With Regards, Amit Kapila.
Re: Fix a comment error in logicalrep_write_typ()
Thank you for review! The commitfest link for tracking: https://commitfest.postgresql.org/49/5121/ -- Regards, ChangAo Chen
Re: Fix a comment error in logicalrep_write_typ()
On Thu, Jul 11, 2024 at 12:46 PM cca5507 wrote: > > - /* use Oid as relation identifier */ > + /* use Oid as type identifier */ > pq_sendint32(out, typoid); > > I think it must be "type identifier" rather than "relation identifier". > LGTM. -- With Regards, Amit Kapila.
Fix a comment error in logicalrep_write_typ()
Hi, - /* use Oid as relation identifier */ + /* use Oid as type identifier */ pq_sendint32(out, typoid); I think it must be "type identifier" rather than "relation identifier". -- Regards, ChangAo Chen 0001-Fix-a-comment-error-in-logicalrep_write_typ.patch Description: Binary data
Re: Fix a comment on PQcancelErrorMessage
On 2024-Jul-04, Yugo NAGATA wrote: > On Thu, 4 Jul 2024 11:06:03 +0200 > Jelte Fennema-Nio wrote: > > > On Thu, 4 Jul 2024 at 06:46, Yugo NAGATA wrote: > > > Attached is a small patch to fix a comment on PQcancelErrorMessage. > > > > Oops, copy paste mistake on my part I guess. New comment LGTM > > Thank you for your comments. > > I made a trivial change that fixes the line break position in keeping with > the surrounding comments. Ugh, mea culpa for failing to notice -- thank you, pushed. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Learn about compilers. Then everything looks like either a compiler or a database, and now you have two problems but one of them is fun." https://twitter.com/thingskatedid/status/1456027786158776329
Re: Fix a comment on PQcancelErrorMessage
On Thu, 4 Jul 2024 11:06:03 +0200 Jelte Fennema-Nio wrote: > On Thu, 4 Jul 2024 at 06:46, Yugo NAGATA wrote: > > Attached is a small patch to fix a comment on PQcancelErrorMessage. > > Oops, copy paste mistake on my part I guess. New comment LGTM Thank you for your comments. I made a trivial change that fixes the line break position in keeping with the surrounding comments. Regards, Yugo Nagata -- Yugo NAGATA diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c index 9562a7fe44..213a6f43c2 100644 --- a/src/interfaces/libpq/fe-cancel.c +++ b/src/interfaces/libpq/fe-cancel.c @@ -299,7 +299,8 @@ PQcancelSocket(const PGcancelConn *cancelConn) /* * PQcancelErrorMessage * - * Get the socket of the cancel connection. + * Returns the error message most recently generated by an operation on the + * cancel connection. */ char * PQcancelErrorMessage(const PGcancelConn *cancelConn)
Re: Fix a comment on PQcancelErrorMessage
On Thu, 4 Jul 2024 at 06:46, Yugo NAGATA wrote: > Attached is a small patch to fix a comment on PQcancelErrorMessage. Oops, copy paste mistake on my part I guess. New comment LGTM
Re: Fix a comment on PQcancelErrorMessage
> Hi, > > Attached is a small patch to fix a comment on PQcancelErrorMessage. > > It was accidentally "Get the socket of the cancel connection." > I rewrote it to "Returns the error message most recently generated > by an operation on the cancel connection." Good catch. The proposed message matches the PQCancelErrorMessage() documentation and looks good. Best reagards, -- Tatsuo Ishii SRA OSS LLC English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
Fix a comment on PQcancelErrorMessage
Hi, Attached is a small patch to fix a comment on PQcancelErrorMessage. It was accidentally "Get the socket of the cancel connection." I rewrote it to "Returns the error message most recently generated by an operation on the cancel connection." Regards, Yugo Nagata -- Yugo NAGATA diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c index 9562a7fe44..e7621a5a61 100644 --- a/src/interfaces/libpq/fe-cancel.c +++ b/src/interfaces/libpq/fe-cancel.c @@ -299,7 +299,8 @@ PQcancelSocket(const PGcancelConn *cancelConn) /* * PQcancelErrorMessage * - * Get the socket of the cancel connection. + * Returns the error message most recently generated by an + * operation on the cancel connection. */ char * PQcancelErrorMessage(const PGcancelConn *cancelConn)
Re: Fix a comment in basic_archive about NO_INSTALLCHECK
On Mon, Dec 18, 2023 at 11:28:46AM +0530, Bharath Rupireddy wrote: > Reworded the comment a bit and attached the v2 patch. Forgot about this one, thanks! I've simplified it a bit and applied it. -- Michael signature.asc Description: PGP signature
Re: Fix a comment in basic_archive about NO_INSTALLCHECK
On Thu, Apr 6, 2023 at 9:26 AM Michael Paquier wrote: > > On Mon, Apr 03, 2023 at 08:56:10AM +0530, Bharath Rupireddy wrote: > > It looks like comments in make file and meson file about not running > > basic_archive tests in NO_INSTALLCHECK mode are wrong. The comments say the > > module needs to be loaded via shared_preload_libraries=basic_archive, but > > it actually doesn't. The custom file needs archive related parameters and > > wal_level=replica. Here's a patch correcting that comment. > > Saying that, updating the comments about the dependency with > archive_library and the module's GUC is right. Reworded the comment a bit and attached the v2 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com v2-0001-Fix-a-comment-in-basic_archive-about-NO_INSTALLCH.patch Description: Binary data
Re: Should fix a comment referring to stats collector?
On Mon, Aug 1, 2022 at 09:05:45PM +0900, torikoshia wrote: > On 2022-07-30 02:53, Alvaro Herrera wrote: > > > I don't think this refers to the statistics collector process; I > > understand it to refer to ANALYZE that captures the data being stored. > > Thanks for the explanation! > > > Maybe it should just say "K may be chosen at ANALYZE time". > > It seems clearer than current one. Change made in master. -- Bruce Momjian https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Re: Fix a comment in paraminfo_get_equal_hashops
On Fri, 4 Aug 2023 at 18:48, Richard Guo wrote: > As stated in [1], there is a typo in the comment in > paraminfo_get_equal_hashops. > [1] > https://www.postgresql.org/message-id/cambws49dehrpe8pom_k39r2uosaozcg+y0b5a8tf7vw3uvr...@mail.gmail.com Thanks. Pushed. David
Fix a comment in paraminfo_get_equal_hashops
As stated in [1], there is a typo in the comment in
paraminfo_get_equal_hashops.
foreach(lc, innerrel->lateral_vars)
{
Node *expr = (Node *) lfirst(lc);
TypeCacheEntry *typentry;
/* Reject if there are any volatile functions in PHVs */
if (contain_volatile_functions(expr))
{
list_free(*operators);
list_free(*param_exprs);
return false;
}
The expressions in RelOptInfo.lateral_vars are not necessarily from
PHVs. For instance
explain (costs off)
select * from t t1 join
lateral (select * from t t2 where t1.a = t2.a offset 0) on true;
QUERY PLAN
--
Nested Loop
-> Seq Scan on t t1
-> Memoize
Cache Key: t1.a
Cache Mode: binary
-> Seq Scan on t t2
Filter: (t1.a = a)
(7 rows)
The lateral Var 't1.a' comes from the lateral subquery.
So attach a trivial patch to fix that.
[1]
https://www.postgresql.org/message-id/cambws49dehrpe8pom_k39r2uosaozcg+y0b5a8tf7vw3uvr...@mail.gmail.com
Thanks
Richard
v1-0001-Fix-a-comment-in-paraminfo_get_equal_hashops.patch
Description: Binary data
Re: Fix a comment in basic_archive about NO_INSTALLCHECK
On Thu, Apr 6, 2023 at 9:26 AM Michael Paquier wrote: > > On Mon, Apr 03, 2023 at 08:56:10AM +0530, Bharath Rupireddy wrote: > > It looks like comments in make file and meson file about not running > > basic_archive tests in NO_INSTALLCHECK mode are wrong. The comments say the > > module needs to be loaded via shared_preload_libraries=basic_archive, but > > it actually doesn't. The custom file needs archive related parameters and > > wal_level=replica. Here's a patch correcting that comment. > > Wouldn't it be better to also set shared_preload_libraries in > basic_archive.conf? It is true that the test works fine if setting > only archive_library, which would cause the library with its > _PG_init() to be loaded in the archiver process. However the GUC > basic_archive.archive_directory is missing from the backends. Hm, I think the other backends will still see the value of the GUC without shared_preload_libraries=basic_archive. You can verify it with adding SHOW basic_archive.archive_directory; to basic_archive.sql. The basic_archive library gets loaded by archiver via _PG_init. It's the archiver defining a custom GUC variable which will propagate to all the postgres processes via set_config_option_ext. Therefore, we don't need shared_preload_libraries=basic_archive. #3 0x7f75306406b6 in _PG_init () at basic_archive.c:86 #4 0x562652d0c87c in internal_load_library ( libname=0x5626549102d8 "/home/ubuntu/postgres/tmp_install/home/ubuntu/postgres/inst/lib/basic_archive.so") at dfmgr.c:289 #5 0x562652d0c1e7 in load_external_function (filename=0x562654930698 "basic_archive", funcname=0x562652eca81b "_PG_archive_module_init", signalNotFound=false, filehandle=0x0) at dfmgr.c:116 #6 0x562652a3a400 in LoadArchiveLibrary () at pgarch.c:841 #7 0x562652a39489 in PgArchiverMain () at pgarch.c:256 #8 0x562652a353de in AuxiliaryProcessMain (auxtype=ArchiverProcess) at auxprocess.c:145 #9 0x562652a40b8e in StartChildProcess (type=ArchiverProcess) at postmaster.c:5341 #10 0x562652a3e529 in process_pm_child_exit () at postmaster.c:3072 #11 0x562652a3c329 in ServerLoop () at postmaster.c:1767 #12 0x562652a3bc52 in PostmasterMain (argc=8, argv=0x56265490e1e0) at postmaster.c:1462 #13 0x5626528efbbf in main (argc=8, argv=0x56265490e1e0) at main.c:198 > Saying that, updating the comments about the dependency with > archive_library and the module's GUC is right. Thanks. Any thoughts on the v1 patch attached upthread? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Fix a comment in basic_archive about NO_INSTALLCHECK
On Mon, Apr 03, 2023 at 08:56:10AM +0530, Bharath Rupireddy wrote: > It looks like comments in make file and meson file about not running > basic_archive tests in NO_INSTALLCHECK mode are wrong. The comments say the > module needs to be loaded via shared_preload_libraries=basic_archive, but > it actually doesn't. The custom file needs archive related parameters and > wal_level=replica. Here's a patch correcting that comment. Wouldn't it be better to also set shared_preload_libraries in basic_archive.conf? It is true that the test works fine if setting only archive_library, which would cause the library with its _PG_init() to be loaded in the archiver process. However the GUC basic_archive.archive_directory is missing from the backends. Saying that, updating the comments about the dependency with archive_library and the module's GUC is right. -- Michael signature.asc Description: PGP signature
Fix a comment in basic_archive about NO_INSTALLCHECK
Hi,
It looks like comments in make file and meson file about not running
basic_archive tests in NO_INSTALLCHECK mode are wrong. The comments say the
module needs to be loaded via shared_preload_libraries=basic_archive, but
it actually doesn't. The custom file needs archive related parameters and
wal_level=replica. Here's a patch correcting that comment.
Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From a3d9c1e6e1f080403b48232c07f136c173865106 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy
Date: Sat, 1 Apr 2023 08:31:52 +
Subject: [PATCH v1] Fix a comment in basic_archive about NO_INSTALLCHECK
---
contrib/basic_archive/Makefile| 5 +++--
contrib/basic_archive/meson.build | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/contrib/basic_archive/Makefile b/contrib/basic_archive/Makefile
index 55d299d650..9216a3060b 100644
--- a/contrib/basic_archive/Makefile
+++ b/contrib/basic_archive/Makefile
@@ -5,8 +5,9 @@ PGFILEDESC = "basic_archive - basic archive module"
REGRESS = basic_archive
REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/basic_archive/basic_archive.conf
-# Disabled because these tests require "shared_preload_libraries=basic_archive",
-# which typical installcheck users do not have (e.g. buildfarm clients).
+# Disabled because these tests require wal_level = replica and archive related
+# parameters to be enabled, which typical installcheck users do not have
+# (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
ifdef USE_PGXS
diff --git a/contrib/basic_archive/meson.build b/contrib/basic_archive/meson.build
index bc1380e6f6..0753b07bf5 100644
--- a/contrib/basic_archive/meson.build
+++ b/contrib/basic_archive/meson.build
@@ -27,8 +27,9 @@ tests += {
'regress_args': [
'--temp-config', files('basic_archive.conf'),
],
-# Disabled because these tests require "shared_preload_libraries=basic_archive",
-# which typical runningcheck users do not have (e.g. buildfarm clients).
+# Disabled because these tests require wal_level = replica and archive related
+# parameters to be enabled, which typical installcheck users do not have
+# (e.g. buildfarm clients).
'runningcheck': false,
},
}
--
2.34.1
Re: Fix a comment in WalSnd structure
On Fri, Aug 19, 2022 at 05:40:40PM +0530, Bharath Rupireddy wrote: > WalSnd structure mutex is being used to protect all the variables of > that structure, not just 'variables shown above' [1]. A tiny patch > attached to fix the comment. Yep, walsender.c tells the same story, aka that replyTime and latch are updated with the spinlock taken. I'll go update the comment, and you suggestion sounds fine to me. -- Michael signature.asc Description: PGP signature
Fix a comment in WalSnd structure
Hi, WalSnd structure mutex is being used to protect all the variables of that structure, not just 'variables shown above' [1]. A tiny patch attached to fix the comment. Thoughts? [1] diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h index c14888e493..9c61f92c44 100644 --- a/src/include/replication/walsender_private.h +++ b/src/include/replication/walsender_private.h @@ -65,7 +65,7 @@ typedef struct WalSnd */ int sync_standby_priority; - /* Protects shared variables shown above. */ + /* Protects shared variables in this structure. */ slock_t mutex; -- Bharath Rupireddy RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/ From 58ef0c3165a9a12718af6173d42479c6e8a756de Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 17 Aug 2022 11:42:51 + Subject: [PATCH v1] Fix a comment in WalSnd structure WalSnd structure mutex is being used to protect all the variables of that structure, not just 'variables shown above'. Correct the comment. --- src/include/replication/walsender_private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h index c14888e493..9c61f92c44 100644 --- a/src/include/replication/walsender_private.h +++ b/src/include/replication/walsender_private.h @@ -65,7 +65,7 @@ typedef struct WalSnd */ int sync_standby_priority; - /* Protects shared variables shown above. */ + /* Protects shared variables in this structure. */ slock_t mutex; /* -- 2.34.1
Re: Should fix a comment referring to stats collector?
On 2022-07-30 02:53, Alvaro Herrera wrote: I don't think this refers to the statistics collector process; I understand it to refer to ANALYZE that captures the data being stored. Thanks for the explanation! Maybe it should just say "K may be chosen at ANALYZE time". It seems clearer than current one. -- Regards, -- Atsushi Torikoshi NTT DATA CORPORATION
Re: Should fix a comment referring to stats collector?
On 2022-Jul-29, torikoshia wrote: > Statistics collector has been removed since 5891c7a8ed8f2d3d5, but there was > a comment referring 'statistics collector' in pg_statistic.h. > > > Note that since the arrays are variable-size, K may be chosen by the > > statistics collector. > > Should it be modified to 'cumulative statistics system' like manual on > monitoring stats[1]? I don't think this refers to the statistics collector process; I understand it to refer to ANALYZE that captures the data being stored. Maybe it should just say "K may be chosen at ANALYZE time". -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Nadie está tan esclavizado como el que se cree libre no siéndolo" (Goethe)
Should fix a comment referring to stats collector?
Hi, Statistics collector has been removed since 5891c7a8ed8f2d3d5, but there was a comment referring 'statistics collector' in pg_statistic.h. Note that since the arrays are variable-size, K may be chosen by the statistics collector. Should it be modified to 'cumulative statistics system' like manual on monitoring stats[1]? Its title has changed from 'statistics collector' to 'cumulative statistics system'. [1] https://www.postgresql.org/docs/current/monitoring-stats.html -- Regards, -- Atsushi Torikoshi NTT DATA CORPORATIONFrom 5be784278e8e7aeeeadf60a772afccda7b59e6e4 Mon Sep 17 00:00:00 2001 From: Atsushi Torikoshi Date: Fri, 29 Jul 2022 21:34:15 +0900 Subject: [PATCH v1] Modified a comment referring to stats collector. As statistics collector has been removed since 5891c7a8ed8f2d3d5, comments refering to statistics collector should be modified to cumulative statistics system. --- src/include/catalog/pg_statistic.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/include/catalog/pg_statistic.h b/src/include/catalog/pg_statistic.h index cdf7448139..d07fd2f93f 100644 --- a/src/include/catalog/pg_statistic.h +++ b/src/include/catalog/pg_statistic.h @@ -178,9 +178,9 @@ DECLARE_FOREIGN_KEY((starelid, staattnum), pg_attribute, (attrelid, attnum)); * the K most common non-null values appearing in the column, and stanumbers * contains their frequencies (fractions of total row count). The values * shall be ordered in decreasing frequency. Note that since the arrays are - * variable-size, K may be chosen by the statistics collector. Values should - * not appear in MCV unless they have been observed to occur more than once; - * a unique column will have no MCV slot. + * variable-size, K may be chosen by the cumulative statistics system. + * Values should not appear in MCV unless they have been observed to occur + * more than once; a unique column will have no MCV slot. */ #define STATISTIC_KIND_MCV 1 base-commit: 59be1c942a47f6c8a4c47d242200fbbf4be59b88 -- 2.27.0
Re: Fix a comment in worker.c
On Fri, Feb 18, 2022 at 1:48 PM Amit Kapila wrote:
>
> On Wed, Feb 16, 2022 at 4:56 PM Amit Kapila wrote:
> >
> > On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada
> > wrote:
> > >
> > > While reading the code, I realized that the second sentence of the
> > > following comment in worker.c is not correct:
> > >
> > >/*
> > > * Exit if the subscription was disabled. This normally should not
> > > happen
> > > * as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
> > > */
> > > if (!newsub->enabled)
> > > {
> > > ereport(LOG,
> > > (errmsg("logical replication apply worker for
> > > subscription \"%s\" will "
> > > "stop because the subscription was disabled",
> > > MySubscription->name)));
> > >
> > > proc_exit(0);
> > > }
> > >
> > > IIUC the apply worker normally exits here when the subscription is
> > > disabled since we don't stop the apply worker during ALTER
> > > SUBSCRIPTION DISABLE. I've attached a patch to remove it.
> > >
> >
> > Yes, I also have the same understanding. Your patch LGTM. I'll push
> > this unless someone thinks otherwise.
> >
>
> Pushed.
Thanks!
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
Re: Fix a comment in worker.c
On Wed, Feb 16, 2022 at 4:56 PM Amit Kapila wrote:
>
> On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada wrote:
> >
> > While reading the code, I realized that the second sentence of the
> > following comment in worker.c is not correct:
> >
> >/*
> > * Exit if the subscription was disabled. This normally should not happen
> > * as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
> > */
> > if (!newsub->enabled)
> > {
> > ereport(LOG,
> > (errmsg("logical replication apply worker for
> > subscription \"%s\" will "
> > "stop because the subscription was disabled",
> > MySubscription->name)));
> >
> > proc_exit(0);
> > }
> >
> > IIUC the apply worker normally exits here when the subscription is
> > disabled since we don't stop the apply worker during ALTER
> > SUBSCRIPTION DISABLE. I've attached a patch to remove it.
> >
>
> Yes, I also have the same understanding. Your patch LGTM. I'll push
> this unless someone thinks otherwise.
>
Pushed.
--
With Regards,
Amit Kapila.
Re: Fix a comment in worker.c
On Wed, Feb 16, 2022 at 5:57 AM Masahiko Sawada wrote:
>
> While reading the code, I realized that the second sentence of the
> following comment in worker.c is not correct:
>
>/*
> * Exit if the subscription was disabled. This normally should not happen
> * as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
> */
> if (!newsub->enabled)
> {
> ereport(LOG,
> (errmsg("logical replication apply worker for
> subscription \"%s\" will "
> "stop because the subscription was disabled",
> MySubscription->name)));
>
> proc_exit(0);
> }
>
> IIUC the apply worker normally exits here when the subscription is
> disabled since we don't stop the apply worker during ALTER
> SUBSCRIPTION DISABLE. I've attached a patch to remove it.
>
Yes, I also have the same understanding. Your patch LGTM. I'll push
this unless someone thinks otherwise.
--
With Regards,
Amit Kapila.
Fix a comment in worker.c
Hi,
While reading the code, I realized that the second sentence of the
following comment in worker.c is not correct:
/*
* Exit if the subscription was disabled. This normally should not happen
* as the worker gets killed during ALTER SUBSCRIPTION ... DISABLE.
*/
if (!newsub->enabled)
{
ereport(LOG,
(errmsg("logical replication apply worker for
subscription \"%s\" will "
"stop because the subscription was disabled",
MySubscription->name)));
proc_exit(0);
}
IIUC the apply worker normally exits here when the subscription is
disabled since we don't stop the apply worker during ALTER
SUBSCRIPTION DISABLE. I've attached a patch to remove it.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
fix_comment_in_worker.c.patch
Description: Binary data
Re: fix a comment
On Sat, Apr 24, 2021 at 11:43 AM Michael Paquier wrote: > > On Fri, Apr 23, 2021 at 07:03:40AM +, wangyu...@fujitsu.com wrote: > > Agree. Please see the v2 patch which delete the number in comment. > > Indeed, this set of comments became a bit obsolete after 1375422, as > you saied upthread. This simplification looks fine to me, so > applied. I am in a mood for such patches since yesterday.. :) Thank you ! Regards, Amul
Re: fix a comment
On Fri, Apr 23, 2021 at 07:03:40AM +, wangyu...@fujitsu.com wrote: > Agree. Please see the v2 patch which delete the number in comment. Indeed, this set of comments became a bit obsolete after 1375422, as you saied upthread. This simplification looks fine to me, so applied. I am in a mood for such patches since yesterday.. -- Michael signature.asc Description: PGP signature
RE: fix a comment
Hi, Amul Thank you for reviewing. > How about simply removing these numbering? Agree. Please see the v2 patch which delete the number in comment. Best wishes Yukun Wang -Original Message- From: Amul Sul Sent: Friday, April 23, 2021 3:51 PM To: Wang, Yukun/王 俞坤 Cc: pgsql-hack...@postgresql.org Subject: Re: fix a comment On Fri, Apr 23, 2021 at 12:12 PM wangyu...@fujitsu.com wrote: > > Hi, Hackers: > > In function ExecGetTriggerResultRel, we can see comments: > > > /* First, search through the query result relations */ ... > > /* > > * Third, search through the result relations that were created > > during > > * tuple routing, if any. > > */ > > But the 'Second' was deleted since commit 1375422c78. > > Update the 'Third' to 'Second', please see the attachment. > > Thoughts? > Well yes, looks good to me. How about simply removing these numbering? Regards, Amul fix-a-comment.diff Description: fix-a-comment.diff
Re: fix a comment
On Fri, Apr 23, 2021 at 12:12 PM wangyu...@fujitsu.com wrote: > > Hi, Hackers: > > In function ExecGetTriggerResultRel, we can see comments: > > > /* First, search through the query result relations */ ... > > /* > > * Third, search through the result relations that were created during > > * tuple routing, if any. > > */ > > But the 'Second' was deleted since commit 1375422c78. > > Update the 'Third' to 'Second', please see the attachment. > > Thoughts? > Well yes, looks good to me. How about simply removing these numbering? Regards, Amul
fix a comment
Hi, Hackers: In function ExecGetTriggerResultRel, we can see comments: > /* First, search through the query result relations */ ... > /* > * Third, search through the result relations that were created during > * tuple routing, if any. > */ But the 'Second' was deleted since commit 1375422c78. Update the 'Third' to 'Second', please see the attachment. Thoughts? Best wishes Yukun Wang fix-a-typo.diff Description: fix-a-typo.diff
Re: Fix a comment in CreateLockFile
On Sun, Dec 8, 2019 at 4:32 PM Amit Kapila wrote: > > On Sun, Dec 8, 2019 at 1:10 PM Hadi Moshayedi wrote: > > > > It seems that explanation for the contents of the pid file has moved to > > pidfile.h, but the comments in CreateLockFile() still point to miscadmin.h. > > > > The attached patch updates those pointers. > > > > Your patch looks correct to me on a quick look. I will take of this tomorrow. > Pushed. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: Fix a comment in CreateLockFile
On Sun, Dec 8, 2019 at 1:10 PM Hadi Moshayedi wrote: > > It seems that explanation for the contents of the pid file has moved to > pidfile.h, but the comments in CreateLockFile() still point to miscadmin.h. > > The attached patch updates those pointers. > Your patch looks correct to me on a quick look. I will take of this tomorrow. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Fix a comment in CreateLockFile
It seems that explanation for the contents of the pid file has moved to pidfile.h, but the comments in CreateLockFile() still point to miscadmin.h. The attached patch updates those pointers. diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 83c9514856..de554e28cf 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -822,7 +822,7 @@ GetUserNameFromId(Oid roleid, bool noerr) * ($DATADIR/postmaster.pid) and Unix-socket-file lockfiles ($SOCKFILE.lock). * Both kinds of files contain the same info initially, although we can add * more information to a data-directory lockfile after it's created, using - * AddToDataDirLockFile(). See miscadmin.h for documentation of the contents + * AddToDataDirLockFile(). See pidfile.h for documentation of the contents * of these lockfiles. * * On successful lockfile creation, a proc_exit callback to remove the @@ -1089,7 +1089,7 @@ CreateLockFile(const char *filename, bool amPostmaster, } /* - * Successfully created the file, now fill it. See comment in miscadmin.h + * Successfully created the file, now fill it. See comment in pidfile.h * about the contents. Note that we write the same first five lines into * both datadir and socket lockfiles; although more stuff may get added to * the datadir lockfile later.
