Re: bk commit into 6.0 tree (og136792:1.2622) BUG#34210

[Øystein Grøvlen]

Chuck Bell wrote:
> Hi,
> 
> My replies below with some deletion. 

Thanks, for all the good advice, Chuck.  I will work to expand the test 
coverage on different drivers.  Do we have a list somewhere of which 
driver is default for the different storage engines?

Some comments inline (I have deleted most of what is not relevant to my 
comments)

>> I will do this.  (I just ask myself why not all this could 
>> have been done within the end_active_trans function).  What 
>> about testing the return values of the functions I call for 
>> errors.  Seems like if the commit fails, this will silently 
>> be ignored.
> 
> Yes, it appears that the errors for these calls are sometimes ignored. If we
> place the guard around it I think we can eliminate most of the 'safe'
> errors. However, examining the ha_autocommit_or_rollback() method reveals it
> either returns 1 or what was passed in as the second argument so checking
> for errors here isn't very helpful. However, that is not the case for
> end_active_trans(). I agree we should check for errors on that method and it
> wouldn't hurt to check for errors on both calls. 

It is not quite clear to me what I should do if an error is returned. 
Just pass it to the caller?  Will the error contain some id that can be 
interpreted at the appropriate level?

>> Why not move the new code outside the "if (table_list)"?  Is 
>> there any 
>> problems with that?
> 
> I don't think there would be any problems because it seems the calls are
> 'safe' but I don't want to take the chance. I think we should leave it
> inside the "if" because I think this problem will not be an issue with
> native drivers and only applies to the default and CS driver. 

I do not understand how we can be sure that native drivers does not 
leave a transaction open.  (Except that the only native driver that 
exists does not care about transactions :-))

Thanks,

-- 
Øystein