May 2nd, 2010
|03:55 am - How on earth is it possible to accept this?|
Somebody should teach me how to accept community contributions.
On Friday I looked at the second version of a community contributed patch. I.e. it was since then re-written. It's Bug#20837, and it again is an infrastructure issue that is identical in all our forks.
While looking at the patch, I encountered 7 issues, which I had to report:
Bug#53340, Bug#53341, Bug#53342, Bug#53343, Bug#53344, Bug#53345, Bug#53346.
(I won't go into the details how the bad piece of code got into 5.0 in the first place, people who were in charge back in 2004 are now all gone.)
My experience suggests that quality is an issue with almost every piece of code that's thrown at us (MariaDB team contributions are an exception, since they are made by ex-MySQLers).
Should a semi-working, semi-documented code be accepted, expecting that there will be more patches?
In that environment it would be impossible to plan for stable versions (if you mean it).
Perhaps the reason for low contributed code quality is that MySQL ecosystem is mostly made by DBAs, not C/C++ engineers. Perhaps through more documentation, standardization, re-engineering we can get better results.
On the other hand, MySQL has a fairly large community of engines. Indeed, we have more than a dozen of fairly significant storage engines. There were no contributions from this group of any significance.
|Date:||May 2nd, 2010 05:53 am (UTC)|| |
i vaguely remember dealing with 'SET TRANSACTION LEVEL' and friends a long, long time ago. actually, it looks like bug #53340 comes directly from my fix for bug #7955 (which also, amusingly enough, implemented it's test case using the bdb engine - i guess we've lost the test for that behavior).
I updated the report, it's the one about using SET TRANSACTION in the middle of a running transaction.
|Date:||May 2nd, 2010 07:54 am (UTC)|| |
Is an issue with almost every piece of code in MySQL period. How can you expect good contributions when you have spaghetti code to start with?
With all due respect, have you actually read our code?
There are lots of claims that MySQL has spaghetti code, but our code quality varies significantly.
This is a part that has been notoriously hard to understand, but if you look at the changes in it between 5.0 and 5.5 you'll be surprised, I think.
I agree that's one of the keys to contributing.
bad code should be rewritten, not documented.
In every comment (or complaint) on your list, I can pinpoint the author and the time of writing. And, as we discussed before, MySQL coding standards got more strict only gradually. Back in 2001 or 2002 they were very, very lax, so all the code from this epoch does not satisfy even the basic criteria.
Besides, dynamic array and hash is an example of orphaned stuff. Will I hurry up documenting these after this blog post? Nope, not my area, and I personally believe we need a completely different container API.
This is not just my view -- as far as I can see, in the server development group few people think that the containers or mysys coding issues are the worst or most important. Lack of modularity has been on the first place so far. And it's what got most attention recently.
My experience in coaching of new server engineers also suggests it's not the most serious issue. Yes, when people find it, they curse, they waste a few hours debugging and asking questions. But soon the problem is overcome and no one bothers to write a patch. Speaking of the patch: I'm looking forward to a patch that provides a template C++ interface to mysys HASH and DYNAMIC ARRAY structures.
No patch in 7 years I've been with the project!
Take the bug in question. The solution does not involve any of mysys. But to write a correct patch, one needs to know semantics of @@session.completion_type, COMMIT AND CHAIN, XA, MySQL two-phase commit algorithm, the notion of MySQL "statement" commit, understand the difference between a transaction started excplicitly, with BEGIN or COMMIT AND CHAIN, or implicitly, with a statement that uses transactional tables, between auto-commit and implicit commit, server flags that indicate one or another.
|Date:||May 3rd, 2010 11:24 am (UTC)|| |
FYI: we(magnus) added a c++-wrapper on top of mysys HASH in 7.0, storage/ndb/include/HashMap.hpp