List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:January 22 2011 6:09pm
Subject:Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385
Bug#57373
View as plain text  
Hi Jorgen,

Sorry for taking so long time.

After looking into it from the storage engine's view I agree with 
Matthias Leich comment [5 Jan 21:20] that MyISAM is not rolled back is 
an acceptable behavior (since it is not transactional). But the error 
code is bad, since it is a very general storage engine error. It would 
be better if it was the same as for InnoDB.

My suggestion would be to simply translate the given handler error to a 
server error like other error codes are handled in handler::print_error.
The error is originally from the MyISAM engine/handler and is returned 
through the partitioning handler).

Which would give the following patch:
=== modified file 'sql/handler.cc'
--- sql/handler.cc	2011-01-11 11:09:54 +0000
+++ sql/handler.cc	2011-01-22 17:41:21 +0000
@@ -2657,6 +2657,7 @@
      break;
    case HA_ERR_KEY_NOT_FOUND:
    case HA_ERR_NO_ACTIVE_RECORD:
+  case HA_ERR_RECORD_DELETED:
    case HA_ERR_END_OF_FILE:
      textno=ER_KEY_NOT_FOUND;
      break;


And the error/warnings returned will then be:
mysqltest: At line 11: query 'UPDATE t1 AS A NATURAL JOIN t1 B SET A.a = 
2, B.b = 3' failed: 1032: Can't find record in 't1'

Warnings from just before the error:
Error 1032 Can't find record in 't1'
Error 1105 An error occured in multi-table update

For both MyISAM and InnoDB.

I think this is an acceptable behavior for this 'almost meaningless and 
theoretic interest only' query ;)

I.e give the error "Can't find record in 't1'" and if one checks the 
warnings like 'SHOW WARNINGS' one will also get "An error occured in 
multi-table update"

To complete my proposed change, a test case with both MyISAM and InnoDB 
would be good.

Please feel free to reassign to me...

Regarding fixing unnecessary work I think in this case it is not needed, 
since it is a 'bad' query anyway, and I think that there might be 
working queries that changes the PK/partitioning columns so that the 
records stay within the same partitions.

Regards
Mattias



On 2011-01-17 10.26, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-5.5/ based on
> revid:john.embretsen@stripped
>
>   3256 Jorgen Loland	2011-01-17
>        Bug#55385: UPDATE statement throws an error, but still updates the
>                   table entries
>        Bug#57373: Multi update+InnoDB reports ER_KEY_NOT_FOUND if a table
>                   is updated twice
>
>        Partitioning fails if multi-update updates the same partitioned
>        table twice and the partitioning key is updated. This is
>        because updates on the first table may move records from one
>        partition to another, and update on the second table will fail
>        to locate the records to update due to this.
>
>        In InnoDB (BUG#57373), the result was that updates on the
>        first table were performed. The transaction was then aborted
>        once updates on the second table failed to locate records.
>        Error ER_KEY_NOT_FOUND was returned. Problem: unneccessary work
>        was performed (update + abort) and misleading error message
>        returned.
>
>        In MyISAM (BUG#55385), the result was that updates on the first
>        table were performed. The transaction was then stopped once
>        updates on the second table failed to locate records. Error
>        "Got error 134 from storage engine" was returned. However,
>        since MyISAM is unable to abort, the updates on the first
>        table were still in effect. Problem: misleading error message
>        and half-performed transaction.
>
>        The fix is to chech if multi-table update will
>         a) update the same partition twice, and
>         b) at least one of these will update the partitioning key
>            and thereby risk moving records to another partition.
>        If this is the case, a meaningful error message is issued
>        before any update work has been done.
>       @ mysql-test/r/partition.result
>          Add test for bugs 55385 and 57373.
>       @ mysql-test/t/partition.test
>          Add test for bugs 55385 and 57373.
>       @ sql/share/errmsg-utf8.txt
>          New error message for multi-table update where the same partition is updated
> twice.
>       @ sql/sql_update.cc
>          Check if multi-table update is about to update the same partitioned table
> twice and issue error.
>
>      modified:
>        mysql-test/r/partition.result
>        mysql-test/t/partition.test
>        sql/share/errmsg-utf8.txt
>        sql/sql_update.cc

[snip...]
Thread
bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385 Bug#57373Jorgen Loland17 Jan
  • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Jon Olav Hauglid17 Jan
    • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Jorgen Loland18 Jan
  • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Mattias Jonsson22 Jan
    • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Jon Olav Hauglid11 Feb
      • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Mattias Jonsson11 Feb
        • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Jorgen Loland11 Feb
        • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385Bug#57373Jorgen Loland14 Feb