On Wed, Sep 20, 2017 at 9:27 PM, Amit Khandekar <amitdkhan...@gmail.com> wrote:
> On 20 September 2017 at 00:06, Robert Haas <robertmh...@gmail.com> wrote: > > On Fri, Sep 15, 2017 at 7:25 AM, Amit Khandekar <amitdkhan...@gmail.com> > wrote: > >> [ new patch ] > 86 - (event == TRIGGER_EVENT_UPDATE && !trigdesc->trig_update_after_row)) 87 + (event == TRIGGER_EVENT_UPDATE && !trigdesc->trig_update_after_row) || 88 + (event == TRIGGER_EVENT_UPDATE && (oldtup == NULL || newtup == NULL))) 89 return; 90 } Either of oldtup or newtup will be valid at a time & vice versa. Can we improve this check accordingly? For e.g.: (event == TRIGGER_EVENT_UPDATE && )(HeapTupleIsValid(oldtup) ^ ItemPointerIsValid(newtup))))) 247 248 + /* 249 + * EDB: In case this is part of update tuple routing, put this row into the 250 + * transition NEW TABLE if we are capturing transition tables. We need to 251 + * do this separately for DELETE and INSERT because they happen on 252 + * different tables. 253 + */ 254 + if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_ capture) 255 + ExecARUpdateTriggers(estate, resultRelInfo, NULL, 256 + NULL, 257 + tuple, 258 + NULL, 259 + mtstate->mt_transition_capture); 260 + 261 list_free(recheckIndexes); 267 268 + /* 269 + * EDB: In case this is part of update tuple routing, put this row into the 270 + * transition OLD TABLE if we are capturing transition tables. We need to 271 + * do this separately for DELETE and INSERT because they happen on 272 + * different tables. 273 + */ 274 + if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_ capture) 275 + ExecARUpdateTriggers(estate, resultRelInfo, tupleid, 276 + oldtuple, 277 + NULL, 278 + NULL, 279 + mtstate->mt_transition_capture); 280 + Initially, I wondered that why can't we have above code right after ExecInsert() & ExecIDelete() in ExecUpdate respectively? We can do that for ExecIDelete() but not easily in the ExecInsert() case, because ExecInsert() internally searches the correct partition's resultRelInfo for an insert and before returning to ExecUpdate resultRelInfo is restored to the old one. That's why current logic seems to be reasonable for now. Is there anything that we can do? Regards, Amul
