Re: bk commit into 5.1 tree (anozdrin:1.2575) BUG#24923

Hi Marc,

thanks for review.

On Thursday 11 October 2007 19:26, Marc Alff wrote:
> 1)
>
> The result for the test sp.test is missing,
> I can't see what the actual result is

fixed.

> 2)
>
> The test is incomplete:
> - a SHOW CREATE FUNCTION would be great,
> to verify that the function was stored properly
>
> - invoking a function that returns NULL is not testing enough:
> returning actual values of the enum (say, 1 call to return the first value,
> 1 other call to return the last value) would verify that the enum values
> themselves are not truncated.

fixed.

> 3)
>
> The database installation scripts have changed for CREATE TABLE ...
> mysql.proc,
> and this will fix the bug for a new 5.1 installation.
>
> What about an upgrade from 5.0 to 5.1 ?
> I expect to see at some point, an ALTER TABLE mysql.proc ...

fixed.

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

> Has the upgrade scenario be considered ?

AFIAK, such tests are outside our test suite. I tested this manually.

> 4)
>
> According to the CSET comments, a truncation does not cause
> CREATE FUNCTION to fail, it's only a warning.

Yes, that's correct.

> If the function code is not stored properly (hence my request for SHOW
> CREATE FUNCTION),
> I think the CREATE should fail, not produce bad data

I don't get how it is related with SHOW CREATE FUNCTION, but nevermind. Fixed.

> 5)
>
> This is minor, but I noticed this coding style for test scripts:

[cut]

Do you need more help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related lists.mysql.com Links announce benchmarks bugs certification cluster commits dotnet gui-tools internals java maxdb myodbc mysql mysqldoc packagers perl plusplus replication win32 Request more information about Pantek

> What concerns me much more is that this style,
> if propagated to the code base, has a pervert effect on merges:
>
> In the code above, the fragment:

[cut]

> actually occupy 11 lines in the source code,
> while not providing any information what would allow a diff / merge tool
> to make the difference between this fragment in bug A and the same in bug B.
>
> My concern is that, if we code like this, we will start having merge
> problems where the wrong code is applied to the wrong place by the merge
> tool, since the context diffs are generated with far less that 11 lines of
> context, and will get confused.

Point taken, but I think that test files are mainly merged manually anyway (by copying new test cases/results from one tree, to another). Auto-merge didn't work for me, sometimes even for trivial cases. So, I think, this is not a big issue. However, I'll think of it more. Hopefully, we'll find a solution, that would satisfy both of us.

-- 
Alexander Nozdrin, Software Developer
MySQL AB, Moscow, Russia, www.mysql.com

-- 
MySQL Code Commits Mailing List
For list archives: 
http://lists.mysql.com/commits
To unsubscribe:    
http://lists.mysql.com/commits?unsub=lists@pantek.com