Hi, On December 7, 2018 11:44:17 AM PST, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: >On 2018-Dec-07, Andres Freund wrote: > >> Hi, >> >> On 2018-12-07 16:20:06 -0300, Alvaro Herrera wrote: >> > case TBLOCK_BEGIN: >> > + s->rollbackScope = XactRollbackScope; >> > s->blockState = TBLOCK_INPROGRESS; >> > + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) >> > + { >> > + PushTransaction(); >> > + s = CurrentTransactionState; /* changed by >> > push */ >> > + s->name = >> > MemoryContextStrdup(TopTransactionContext, "pg >internal"); >> > + StartSubTransaction(); >> > + s->blockState = TBLOCK_SUBINPROGRESS; >> > + } >> >> Isn't this going to be performing ridiculously bad, to the point of >> being not much but a trap for users? > >If you bulk-load with INSERT under this behavior, yeah it'll create >lots >of subtransactions, with the obvious downsides -- eating lots of xids >for one. I think the answer to that is "don't do that". However, if >you want to do 100k inserts and have the 10 bogus of those fail cleanly >(and not abort the other 99990), that's clearly this patch's intention >> I can see the feature being useful, but I don't think we should >accept a >> feature that'll turn bulkloading with insert into a server shutdown >> scenario. > >Hm.
Isn't the realistic scenario for migrations that people will turn this feature on on a more global basis? If they need to do per transaction choices, that makes this much less useful for easy migrations. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.