Re: inserts into partitioned table may cause crash

2018-03-19 Thread Etsuro Fujita
(2018/03/20 9:34), Amit Langote wrote: On 2018/03/20 5:54, Alvaro Herrera wrote: Etsuro Fujita wrote: Thanks for the updated patches! I think the patches are in good shape, but I did a bit of editorial things; added a bit more comments for ExecPrepareTupleRouting and adjusted regression test

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Amit Langote
On 2018/03/20 5:54, Alvaro Herrera wrote: > Etsuro Fujita wrote: > >> Thanks for the updated patches! I think the patches are in good shape, but >> I did a bit of editorial things; added a bit more comments for >> ExecPrepareTupleRouting and adjusted regression test stuff to match the >>

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Alvaro Herrera
Etsuro Fujita wrote: > Thanks for the updated patches! I think the patches are in good shape, but > I did a bit of editorial things; added a bit more comments for > ExecPrepareTupleRouting and adjusted regression test stuff to match the > existing ones. Attached are the updated patches for HEAD

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Robert Haas
On Mon, Mar 19, 2018 at 9:38 AM, Alvaro Herrera wrote: > Etsuro Fujita wrote: >> Thanks for the updated patches! I think the patches are in good shape, but >> I did a bit of editorial things; added a bit more comments for >> ExecPrepareTupleRouting and adjusted

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Alvaro Herrera
Etsuro Fujita wrote: > Thanks for the updated patches! I think the patches are in good shape, but > I did a bit of editorial things; added a bit more comments for > ExecPrepareTupleRouting and adjusted regression test stuff to match the > existing ones. Attached are the updated patches for HEAD

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Etsuro Fujita
(2018/03/19 17:48), Amit Langote wrote: On 2018/03/16 20:37, Etsuro Fujita wrote: (2018/03/14 17:25), Etsuro Fujita wrote: (2018/03/14 14:54), Amit Langote wrote: Btw, I noticed that the patches place ExecPrepareTupleRouting (both the declaration and the definition) at different relative

Re: inserts into partitioned table may cause crash

2018-03-19 Thread Amit Langote
Fujita-san, Thanks for the review. On 2018/03/16 20:37, Etsuro Fujita wrote: > (2018/03/14 17:25), Etsuro Fujita wrote: >> (2018/03/14 14:54), Amit Langote wrote: >>> Btw, I noticed that the patches place ExecPrepareTupleRouting (both the >>> declaration and the definition) at different relative

Re: inserts into partitioned table may cause crash

2018-03-14 Thread Thomas Munro
On Wed, Mar 14, 2018 at 9:25 PM, Etsuro Fujita wrote: > (2018/03/14 14:54), Amit Langote wrote: >> On 2018/03/06 21:26, Etsuro Fujita wrote: >>> if (mtstate->mt_transition_capture != NULL) >>> { >>> if (resultRelInfo->ri_TrigDesc&& >>>

Re: inserts into partitioned table may cause crash

2018-03-14 Thread Amit Langote
On 2018/03/14 17:35, Etsuro Fujita wrote: > (2018/03/14 17:25), Etsuro Fujita wrote: >> (2018/03/14 14:54), Amit Langote wrote: >>> Sorry that this may be nitpicking that I should've brought up before, but >>> doesn't ExecPrepareTupleRouting do all the work that's needed for routing >>> a tuple

Re: inserts into partitioned table may cause crash

2018-03-14 Thread Etsuro Fujita
(2018/03/14 17:25), Etsuro Fujita wrote: (2018/03/14 14:54), Amit Langote wrote: Sorry that this may be nitpicking that I should've brought up before, but doesn't ExecPrepareTupleRouting do all the work that's needed for routing a tuple and hence isn't the name a bit misleading? Maybe,

Re: inserts into partitioned table may cause crash

2018-03-14 Thread Etsuro Fujita
(2018/03/14 14:54), Amit Langote wrote: On 2018/03/06 21:26, Etsuro Fujita wrote: One thing I notice while working on this is this in ExecInsert/CopyFrom, /* * If we're capturing transition tuples, we might need to convert from the * partition rowtype to parent rowtype.

Re: inserts into partitioned table may cause crash

2018-03-13 Thread Amit Langote
Fujita-san, Thanks for the updates and sorry I couldn't reply sooner. On 2018/03/06 21:26, Etsuro Fujita wrote: > One thing I notice while working on this is this in ExecInsert/CopyFrom, > which I moved to ExecPrepareTupleRouting as-is for the former: > > /* > * If we're capturing

Re: inserts into partitioned table may cause crash

2018-03-11 Thread Etsuro Fujita
(2018/03/09 20:18), Etsuro Fujita wrote: Here are updated patches for PG10 and HEAD. Other changes: * Add regression tests based on your test cases shown upthread I added a little bit more regression tests and revised comments. Please find attached an updated patch. Best regards, Etsuro

Re: inserts into partitioned table may cause crash

2018-03-09 Thread Etsuro Fujita
(2018/03/06 21:26), Etsuro Fujita wrote: One thing I notice while working on this is this in ExecInsert/CopyFrom, which I moved to ExecPrepareTupleRouting as-is for the former: /* * If we're capturing transition tuples, we might need to convert from the * partition rowtype to parent rowtype. */

Re: inserts into partitioned table may cause crash

2018-03-06 Thread Etsuro Fujita
Hi Amit, (2018/03/06 15:28), Amit Langote wrote: On 2018/03/05 22:00, Etsuro Fujita wrote: An alternative fix for this would be to handle the set/reset of estate->es_result_relation_info in a higher level ie, ExecModifyTable, like the attached: Your patch seems like a good cleanup overall,

Re: inserts into partitioned table may cause crash

2018-03-05 Thread Amit Langote
Fujita-san, Thanks for the review. On 2018/03/05 22:00, Etsuro Fujita wrote: > I started reviewing this.  I think the analysis you mentioned upthread > would be correct, but I'm not sure the patch is the right way to go > because I think that exception handling added by the patch throughout >

Re: inserts into partitioned table may cause crash

2018-03-05 Thread Etsuro Fujita
(2018/03/01 21:40), Etsuro Fujita wrote: (2018/02/28 17:36), Amit Langote wrote: Attached a patch to fix that, which would need to be back-patched to 10. Good catch! Will review. I started reviewing this. I think the analysis you mentioned upthread would be correct, but I'm not sure the

Re: inserts into partitioned table may cause crash

2018-03-01 Thread Etsuro Fujita
(2018/02/28 17:36), Amit Langote wrote: I've run into what seems to be a bug in ExecInsert() that causes a crash when inserting multiple rows into a partitioned table that each go into different partitions with different tuple descriptors. Crash occurs if ExecInsert() returns without resetting

Re: inserts into partitioned table may cause crash

2018-02-28 Thread Amit Langote
On 2018/02/28 17:36, Amit Langote wrote: > I've run into what seems to be a bug in ExecInsert() that causes a crash > when inserting multiple rows into a partitioned table that each go into > different partitions with different tuple descriptors. Crash occurs if > ExecInsert() returns without

inserts into partitioned table may cause crash

2018-02-28 Thread Amit Langote
I've run into what seems to be a bug in ExecInsert() that causes a crash when inserting multiple rows into a partitioned table that each go into different partitions with different tuple descriptors. Crash occurs if ExecInsert() returns without resetting estate->es_result_relation_info back to