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.
> 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]
> 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
