Re: bk commit into 5.0 tree (antony:1.2493) BUG#25511

On Mon, 2007-06-25 at 19:09 +0200, Ingo Strüwing wrote:
> Hi Antony,
>
> antony@mysql.com wrote:
> ...
> > ChangeSet@1.2493, 2007-06-22 10:04:09-07:00, antony@ppcg5.local +4 -0
> > Bug#25511
> > "Federated INSERT failures"
> > Federated does not correctly handle "INSERT ... ON DUPLICATE KEY UPDATE"
> > If it is in effect, we must permit mysqld to retry the insert
> > operation. We check if the local primary key definition is adequate
> > to identify a specific row on the remote server by its primary key.
> > This patch builds on Bug29019
> ...
>
> Did you notice Brians comment in the bug report? His proposal seems to
> be the simplest one.

Simplest is to mark bug as "Won't fix".

> Your patch solves the most common problems, but (if I understand
> correctly) it cannot solve all situations. In many cases, a patch like
> this would be the best compromise.
>
> However, my personal discomfort results from different error messages,
> depending on matching primary keys. Especially that a duplicate key
> error can result from an INSERT ... ON DUPLICATE UPDATE.
>
> Also I don't fully understand why you try to check for the primary key
> (only). Other uniques could also cause problems without the primary key
> being duplicate.

In case where a duplicate key occurrs, if the key cannot be extracted, then the server will do a PK based update. If the ON UPDATE condition fails on the 2nd try, then it matters not.

> IMHO the problem could be properly fixed only if we obey what the 5.1
> manual says: "Note When you create the local table is must have an
> identical definition to the remote table."
> (http://dev.mysql.com/doc/refman/5.1/en/federated-create.html). Of
> course this requires that we are able to ensure that. If not, we are
> lost with this issue.
>
> If we can match table definitions, then we should be able to parse the
> error message and find the key in error (number in 5.0 or name in 5.1).
>
> The rest should work "out of the box".
>
> Unfortunately I doubt that we _want_ to spend the effort of matching the
> table definitions (which we would need to do at every open, or at the
> first error like this). So I'd interpret Brians comment as "don't allow
> ON DUPLICATE UPDATE for federated". However, I don't know how to prevent
> this per engine.
>
> A reasonable alternative would be to _always_ return a certain error
> message and document this for federated. IMHO this should work if we
> return -1 as errkey in ::info(), and translate error == HA_WRITE_SKIP
> into the desired message in ha_federated::get_error_message().

Then perhaps a new table flag to indicate that ON DUPLICATE KEY UPDATE is not supported? So that other things don't break, we have 3 options.

1. Mark this bug as "Won't fix"
2. New table flag for disabling "ON DUPLICATE KEY UPDATE".
3. This patch.

To cut out parts of the patch (its pretty much minimal) will result in Bug#29019 regression. The PK check is a quick test which is not in the critical path when there are no errors and provides a good safety net.

Regards,
Antony.

-- 
Antony T Curtis, Senior Software Developer
MySQL Inc, www.mysql.com
SIP: 4468@sip.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

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