Skip to content

Commit

Permalink
Don't ever apply a trigger to the entity that's triggering the update (…
Browse files Browse the repository at this point in the history 
…#94)
  • Loading branch information
@nevali
nevali committed Nov 16, 2016
1 parent 475e1d9 commit 5280c2d
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion twine/generate/triggers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,18 @@ spindle_trigger_apply(SPINDLEENTRY *entry)
{
return 0;
}
rs = sql_queryf(entry->generate->db, "SELECT \"id\", \"flags\" FROM \"triggers\" WHERE \"triggerid\" = %Q", entry->id);
rs = sql_queryf(entry->generate->db, "SELECT \"id\", \"flags\", \"triggerid\" FROM \"triggers\" WHERE \"triggerid\" = %Q", entry->id);
if(!rs)
{
return -1;
}
for(; !sql_stmt_eof(rs); sql_stmt_next(rs))
{
/* Never apply a trigger to the entry itself */
if(!strcmp(sql_stmt_str(rs, 0), sql_stmt_str(rs, 2)))

This comment has been minimized.

Sign in to view

Copy link
@rjpwork

rjpwork Nov 16, 2016

Rather than skipping the matching IDs here, might it be more profitable to add a AND id <> %Q to the query on line 82 and just not fetch them in the first place?

This comment has been minimized.

Sign in to view

Copy link
@nevali

nevali Nov 16, 2016

Author Member

given the small numbers of rows in a trigger result-set, filtering upon receive is likely to be more efficient than giving the query planner more work to do

This comment has been minimized.

Sign in to view

Copy link
@rjpwork

rjpwork Nov 16, 2016

I wouldn't expect it to be much more work if there's already an index on id and indeed it's virtually identical.

spindle=# explain select id, flags, triggerid from triggers where triggerid='4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f';         
                                   QUERY PLAN                                    
---------------------------------------------------------------------------------
 Bitmap Heap Scan on triggers  (cost=4.28..12.74 rows=4 width=36)
   Recheck Cond: (triggerid = '4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f'::uuid)
   ->  Bitmap Index Scan on triggers_triggerid  (cost=0.00..4.28 rows=4 width=0)
         Index Cond: (triggerid = '4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f'::uuid)
(4 rows)

spindle=# explain select id, flags, triggerid from triggers where triggerid='4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f' and id <> triggerid;                    
                                   QUERY PLAN                                    
---------------------------------------------------------------------------------
 Bitmap Heap Scan on triggers  (cost=4.28..12.75 rows=4 width=36)
   Recheck Cond: (triggerid = '4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f'::uuid)
   Filter: (id <> triggerid)
   ->  Bitmap Index Scan on triggers_triggerid  (cost=0.00..4.28 rows=4 width=0)
         Index Cond: (triggerid = '4e6efed6-5e9d-4b3f-b224-3a8aa2cab25f'::uuid)
(5 rows)

This comment has been minimized.

Sign in to view

Copy link
@nevali

nevali Nov 16, 2016

Author Member

in which case, yes — the additional AND clause would indeed be cleaner; I'll amend and roll it into develop
{
continue;
}
flags = (int) sql_stmt_long(rs, 1);
if(!flags)
{
Expand Down

0 comments on commit 5280c2d

Please sign in to comment.