MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Ingo Strüwing Date:June 29 2007 7:53am
Subject:Re: bk commit into 5.0 tree (antony:1.2505) BUG#25511
View as plain text  
Hi Antony,

with the new flag I would have implemented it differently.
In wrte_row(), noticing the new flag is set, I would return with
"Storage engine does not have this option". This would prevent any
attempt to use "ON DUPLICATE KEY UPDATE" with FEDERATED right from the
start. Otherwise an application could run for years until the first
duplicate happens and it starts to fail.

But facing our tight time constraints and thinking your solution is the
second best, I say OK to push from me anyway. If you like, you can
consider my below suggestions regarding comments.

Please do not forget to inform the docs team that "ON DUPLICATE KEY
UPDATE" is silently ignored for FEDERATED until a duplicate happens. On
duplicate, a duplicate key error is returned. In case of a multi-row
insert, the statement breaks at the affected row, leaving previous rows
inserted. Even if a transactional storage engine is used for the remote
table.

antony@stripped wrote:
...
> ChangeSet@stripped, 2007-06-28 13:36:26-07:00, antony@stripped +6 -0
>   Bug#25511
>     "Federated INSERT failures"
>     Federated does not correctly handle "INSERT...ON DUPLICATE KEY UPDATE"
>     However, implementing such support is not reasonably possible without
>     increasing complexity of the storage engine: checking that constraints
>     on remote server match local server and parsing error messages.
>     This patch causes 'ON DUPLICATE KEY' to fail with ER_DUP_KEY message
>     if a conflict occurs and not to fail silently.
...
> +  /*
> +    Inform handler that write_row() should immediately report constraint
> +    violations because a INSERT...ON DUPLICATE KEY UPDATE is in being
> +    performed.
> +  */
> +  HA_EXTRA_INSERT_WITH_UPDATE

For my perception, the first part of the sentence is confusing. I'd have
written: "Inform the handler that an INSERT ... ON DUPLICATE KEY UPDATE
will be executed."

write_row() reports the constraint violation in any case. It is the SQL
layer, which transforms that error into an update if the handler can
return the offended key number.

...
> +  case HA_EXTRA_INSERT_WITH_UPDATE:
> +    insert_dup_update= TRUE;
> +    break;

I'd like to see a comment here, how FEDERATED uses this flag: We create
"INSERT" instead of "INSERT IGNORE" and prevent bulk-insert (which is
handled in Bug#25513 tough), so that we break a bulk-insert at the
offended row and do not continue to insert other rows.

...

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
Thread
bk commit into 5.0 tree (antony:1.2505) BUG#25511antony28 Jun
  • Re: bk commit into 5.0 tree (antony:1.2505) BUG#25511Ingo Strüwing29 Jun