Fix a typo in execReplication.c

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Fix a typo in execReplication.c

Masahiko Sawada
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


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

fix_comment_in_execReplication_c.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Robert Haas
On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <[hidden email]> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Masahiko Sawada
On Thu, Oct 12, 2017 at 5:02 AM, Robert Haas <[hidden email]> wrote:

> On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <[hidden email]> 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


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

fix_brtirg_condition_in_ExecSimpleRelationDelete.patch (828 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Petr Jelinek-4
On 12/10/17 00:52, Masahiko Sawada wrote:

> On Thu, Oct 12, 2017 at 5:02 AM, Robert Haas <[hidden email]> wrote:
>> On Mon, Oct 9, 2017 at 10:59 PM, Masahiko Sawada <[hidden email]> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Robert Haas
On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
<[hidden email]> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Masahiko Sawada
On Thu, Oct 12, 2017 at 11:30 PM, Robert Haas <[hidden email]> wrote:
> On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
> <[hidden email]> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: Fix a typo in execReplication.c

Simon Riggs
On 13 October 2017 at 09:13, Masahiko Sawada <[hidden email]> wrote:
> On Thu, Oct 12, 2017 at 11:30 PM, Robert Haas <[hidden email]> wrote:
>> On Thu, Oct 12, 2017 at 6:55 AM, Petr Jelinek
>> <[hidden email]> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Previous Thread Next Thread