Hi Antony,
Antony T Curtis wrote:
> On Mon, 2007-06-25 at 19:09 +0200, Ingo Strüwing wrote:
...
> 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.
If I understand sql_insert.cc:write_record() correctly, a duplicate key
error on handler::write_row(new_row) is handled by trying to find the
offended unique index, reading old_row based on the unique key value,
and then updating with handler::update_row(new_row, old_row). If the
"key cannot be extracted", then the statement fails.
An offended PK may be the most common problem, but one should easily
find cases where we still fail to perform the update even when the table
definitions match perfectly.
...
>> 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.
IMHO, 3. is not an acceptable fix. Unless we put major effort in
documenting the circumstances in which it works. If I understand your
patch correctly, you check if the column names of the PK match. IMHO
this is insufficient for a safe match. And you do not handle other
unique indexes, which could also be guilty of the duplicate key problems.
Also, I beg you to add my proposal to the list: Always let "ON DUPLICATE
KEY UPDATE" fail for FEDERATED with an error messages that tells "A
duplicate key error happened and FEDERATED cannot turn it into an
update". Of course this does also require some documentation.
With this list please ask the architecture board what to do.
> To cut out parts of the patch (its pretty much minimal) will result in
> Bug#29019 regression.
Please explain. I can't follow.
> 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.
Agree except of the last part "provides a good safety net". IMHO the
check is too limited to call it a "safety net".
I still believe that the only way for a real fix goes via a safe
identification of the offended unique index and a safe match of the
table definitions.
Regards
Ingo
--
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140
