On Mon, 2007-06-25 at 19:09 +0200, Ingo Strüwing wrote:
> Hi Antony,
>
> antony@stripped wrote:
> ...
> > ChangeSet@stripped, 2007-06-22 10:04:09-07:00, antony@stripped +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@stripped
