Horton

Thread

Commits

  1. Merge near-duplicate code in RI triggers.

  1. Repetitive code in RI triggers

    Ildar Musin <i.musin@postgrespro.ru> — 2017-04-10T15:55:04Z

    Hi all,

I was looking through the RI triggers code recently and noticed a few 
almost identical functions, e.g. ri_restrict_upd() and 
ri_restrict_del(). The following patch is an attempt to reduce some of 
repetitive code. Yet there is still room for improvement.

Thanks,
-- 
Ildar Musin
i.musin@postgrespro.ru

  2. Re: Repetitive code in RI triggers

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2017-04-11T01:41:27Z

    On 4/10/17 11:55, Ildar Musin wrote:
> I was looking through the RI triggers code recently and noticed a few 
> almost identical functions, e.g. ri_restrict_upd() and 
> ri_restrict_del(). The following patch is an attempt to reduce some of 
> repetitive code.

That looks like something worth pursuing.  Please add it to the next
commit fest.

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

  3. Re: Repetitive code in RI triggers

    Daniel Gustafsson <daniel@yesql.se> — 2017-09-19T08:09:04Z

    > On 11 Apr 2017, at 03:41, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
> 
> On 4/10/17 11:55, Ildar Musin wrote:
>> I was looking through the RI triggers code recently and noticed a few 
>> almost identical functions, e.g. ri_restrict_upd() and 
>> ri_restrict_del(). The following patch is an attempt to reduce some of 
>> repetitive code.
> 
> That looks like something worth pursuing.  Please add it to the next
> commit fest.

Removing reviewer Maksim Milyutin from patch entry due to inactivity and
community account email bouncing.  Maksim: if you are indeed reviewing this
patch, then please update the community account and re-add to the patch entry.

cheers ./daniel

  4. Re: Repetitive code in RI triggers

    Maksim Milyutin <milyutinma@gmail.com> — 2017-09-26T08:51:26Z

    On 19.09.2017 11:09, Daniel Gustafsson wrote:

> Removing reviewer Maksim Milyutin from patch entry due to inactivity and
> community account email bouncing.  Maksim: if you are indeed reviewing this
> patch, then please update the community account and re-add to the patch entry.
>
> cheers ./daniel

Daniel, thanks for noticing. I have updated my account and re-added to 
the patch entry.

Ildar, your patch is conflicting with the current HEAD of master branch, 
please update it.

-- 
Regards,
Maksim Milyutin

  5. Re: Repetitive code in RI triggers

    Daniel Gustafsson <daniel@yesql.se> — 2017-09-26T08:53:11Z

    > On 26 Sep 2017, at 10:51, Maksim Milyutin <milyutinma@gmail.com> wrote:
> 
> On 19.09.2017 11:09, Daniel Gustafsson wrote:
> 
>> Removing reviewer Maksim Milyutin from patch entry due to inactivity and
>> community account email bouncing.  Maksim: if you are indeed reviewing this
>> patch, then please update the community account and re-add to the patch entry.
>> 
>> cheers ./daniel
> 
> Daniel, thanks for noticing. I have updated my account and re-added to the patch entry.

Great, thanks!

> Ildar, your patch is conflicting with the current HEAD of master branch, please update it.

I’ve changed status to Waiting on Author based on this.

cheers ./daniel

  6. Re: Repetitive code in RI triggers

    Ildar Musin <i.musin@postgrespro.ru> — 2017-10-03T10:48:43Z

    Hi Maksim,

On 26.09.2017 11:51, Maksim Milyutin wrote:
> On 19.09.2017 11:09, Daniel Gustafsson wrote:
>
>> Removing reviewer Maksim Milyutin from patch entry due to inactivity and
>> community account email bouncing.  Maksim: if you are indeed reviewing
>> this
>> patch, then please update the community account and re-add to the
>> patch entry.
>>
>> cheers ./daniel
>
> Daniel, thanks for noticing. I have updated my account and re-added to
> the patch entry.
>
> Ildar, your patch is conflicting with the current HEAD of master branch,
> please update it.
>

Thank you for checking the patch out. Yes, it seems that original code 
was reformatted and this led to merging conflicts. I've fixed that and 
also introduced some minor improvements. The new version is in attachment.

Thanks!

-- 
Ildar Musin
i.musin@postgrespro.ru

  7. Re: Repetitive code in RI triggers

    Ildus Kurbangaliev <i.kurbangaliev@gmail.com> — 2017-11-17T15:05:31Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       tested, failed
Spec compliant:           tested, failed
Documentation:            tested, failed

The patch looks good. It just removes repetitive code and I think it's ready to commit.

The new status of this patch is: Ready for Committer

  8. Re: Repetitive code in RI triggers

    Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru> — 2017-11-17T15:08:53Z

    On Fri, 17 Nov 2017 15:05:31 +0000
Ildus Kurbangaliev <i.kurbangaliev@gmail.com> wrote:

> The following review has been posted through the commitfest
> application: make installcheck-world:  tested, failed
> Implements feature:       tested, failed
> Spec compliant:           tested, failed
> Documentation:            tested, failed
> 
> The patch looks good. It just removes repetitive code and I think
> it's ready to commit.
> 
> The new status of this patch is: Ready for Committer

"tested, failed" should be read as "tested, passed". Forgot to check
them.

-- 
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

  9. Re: [HACKERS] Repetitive code in RI triggers

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-18T21:31:16Z

    Ildar Musin <i.musin@postgrespro.ru> writes:
> [ ri_triggers_v2.patch ]

Pushed with two minor improvements.  I noticed that ri_setdefault could
just go directly to ri_restrict rather than call the two separate triggers
that would end up there anyway; that lets its argument be "TriggerData
*trigdata" for more consistency with the other cases.  Also, this patch
made it very obvious that we were caching identical queries under hash
keys RI_PLAN_RESTRICT_DEL_CHECKREF and RI_PLAN_RESTRICT_UPD_CHECKREF,
so we might as well just use one hash entry for both cases, saving a few
lines of code as well as a lot of cycles.  Likewise in the other two
functions.

			regards, tom lane

  10. Re: [HACKERS] Repetitive code in RI triggers

    Ildar Musin <i.musin@postgrespro.ru> — 2017-11-20T08:53:38Z

    
On 19.11.2017 00:31, Tom Lane wrote:
> Ildar Musin <i.musin@postgrespro.ru> writes:
>> [ ri_triggers_v2.patch ]
>
> Pushed with two minor improvements.  I noticed that ri_setdefault could
> just go directly to ri_restrict rather than call the two separate triggers
> that would end up there anyway; that lets its argument be "TriggerData
> *trigdata" for more consistency with the other cases.  Also, this patch
> made it very obvious that we were caching identical queries under hash
> keys RI_PLAN_RESTRICT_DEL_CHECKREF and RI_PLAN_RESTRICT_UPD_CHECKREF,
> so we might as well just use one hash entry for both cases, saving a few
> lines of code as well as a lot of cycles.  Likewise in the other two
> functions.
>
> 			regards, tom lane
>

Great, thanks

-- 
Ildar Musin
i.musin@postgrespro.ru