Fix a typo in execReplication.c

Started by Masahiko Sawadaover 8 years ago7 messages
#1Masahiko Sawada
sawada.mshk@gmail.com
1 attachment(s)

Hi,

Attached a patch for $subject. ISTM these are mistakes of copy-and-paste.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

fix_comment_in_execReplication_c.patchapplication/octet-stream; name=fix_comment_in_execReplication_c.patchDownload
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 5a75e02..c26420a 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -448,7 +448,7 @@ ExecSimpleRelationUpdate(EState *estate, EPQState *epqstate,
 
 	CheckCmdReplicaIdentity(rel, CMD_UPDATE);
 
-	/* BEFORE ROW INSERT Triggers */
+	/* BEFORE ROW UPDATE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
@@ -509,7 +509,7 @@ ExecSimpleRelationDelete(EState *estate, EPQState *epqstate,
 
 	CheckCmdReplicaIdentity(rel, CMD_DELETE);
 
-	/* BEFORE ROW INSERT Triggers */
+	/* BEFORE ROW DELETE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
 		resultRelInfo->ri_TrigDesc->trig_update_before_row)
 	{
#2Robert Haas
robertmhaas@gmail.com
In reply to: Masahiko Sawada (#1)
Re: Fix a typo in execReplication.c

On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached a patch for $subject. ISTM these are mistakes of copy-and-paste.

Committed, but isn't the code itself wrong as well in the DELETE case?

/* BEFORE ROW DELETE Triggers */
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_before_row)
{
skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
&searchslot->tts_tuple->t_self,
NULL);
}

Why not trig_delete_before_row?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Robert Haas (#2)
1 attachment(s)
Re: Fix a typo in execReplication.c

On Thu, Oct 12, 2017 at 5:02 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached a patch for $subject. ISTM these are mistakes of copy-and-paste.

Committed, but isn't the code itself wrong as well in the DELETE case?

/* BEFORE ROW DELETE Triggers */
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_before_row)
{
skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
&searchslot->tts_tuple->t_self,
NULL);
}

Why not trig_delete_before_row?

Thank you!
I think you're right. It should be trig_delete_before_row and be
back-patched to 10. Attached patch fixes it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

fix_brtirg_condition_in_ExecSimpleRelationDelete.patchtext/x-patch; charset=US-ASCII; name=fix_brtirg_condition_in_ExecSimpleRelationDelete.patchDownload
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index c26420a..fb538c0 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -511,7 +511,7 @@ ExecSimpleRelationDelete(EState *estate, EPQState *epqstate,
 
 	/* BEFORE ROW DELETE Triggers */
 	if (resultRelInfo->ri_TrigDesc &&
-		resultRelInfo->ri_TrigDesc->trig_update_before_row)
+		resultRelInfo->ri_TrigDesc->trig_delete_before_row)
 	{
 		skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
 										   &searchslot->tts_tuple->t_self,
#4Petr Jelinek
petr.jelinek@2ndquadrant.com
In reply to: Masahiko Sawada (#3)
Re: Fix a typo in execReplication.c

On 12/10/17 00:52, Masahiko Sawada wrote:

On Thu, Oct 12, 2017 at 5:02 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Attached a patch for $subject. ISTM these are mistakes of copy-and-paste.

Committed, but isn't the code itself wrong as well in the DELETE case?

/* BEFORE ROW DELETE Triggers */
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_update_before_row)
{
skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
&searchslot->tts_tuple->t_self,
NULL);
}

Why not trig_delete_before_row?

Indeed, that's another copy-pasto.

Thank you!
I think you're right. It should be trig_delete_before_row and be
back-patched to 10. Attached patch fixes it.

Thanks for the patch, looks correct to me.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Robert Haas
robertmhaas@gmail.com
In reply to: Petr Jelinek (#4)
Re: Fix a typo in execReplication.c

On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

Thanks for the patch, looks correct to me.

Committed and back-patched to v10.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Robert Haas (#5)
Re: Fix a typo in execReplication.c

On Thu, Oct 12, 2017 at 11:30 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

Thanks for the patch, looks correct to me.

Committed and back-patched to v10.

Thank you!

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Simon Riggs
simon@2ndquadrant.com
In reply to: Masahiko Sawada (#6)
Re: Fix a typo in execReplication.c

On 13 October 2017 at 09:13, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Oct 12, 2017 at 11:30 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:

Thanks for the patch, looks correct to me.

Committed and back-patched to v10.

Well spotted both of you!

Shows that reading code and correcting comments is useful activity.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers