On Wed, Jan 19, 2022 at 6:26 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > On 2022-Jan-19, Amit Langote wrote: > > BTW, your tweaks patch added this: > > > > + * *inserted_tuple is the tuple that's effectively inserted; > > + * *inserted_destrel is the relation where it was inserted. > > + * These are only set on success. FIXME -- see what happens on > > the "do nothing" cases. > > > > If by "do nothing cases" you mean INSERT ON CONFLICT ... DO NOTHING, > > then I don't think it matters, because the caller in that case would > > be ExecModifyTable() which doesn't care about inserted_tuple and > > inserted_destrel. > > No, I meant a FOR EACH ROW trigger that does RETURN NULL to "abort" the > insertion.
Ah, gotcha. > IIRC in non-partitioned cases it is possibly to break > referential integrity by using those. What I was wondering is whether > you can make this new code crash. insert_destrel would be left set to NULL, which means ExecCrossPartitionUpdateForeignKey() won't get called, because: * NULL insert_destrel means that the move failed to occur, that * is, the update failed, so no need to anything in that case. */ if (insert_destrel && resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_update_after_row) ExecCrossPartitionUpdateForeignKey(mtstate, estate, Moreover, trigger documentation warns of a "possibility of surprising outcomes" if BR triggers are present in partitions that are chosen as destinations of cross-partition updates: "Then all row-level BEFORE INSERT triggers are fired on the destination partition. The possibility of surprising outcomes should be considered when all these triggers affect the row being moved." I suppose the new code shouldn't need to take special care in such cases either. -- Amit Langote EDB: http://www.enterprisedb.com