Hakan,
Overall good changes, using the template and all :)
Patch OK to push, however please see my inline comments anyway...
Hakan Kuecuekyilmaz wrote:
> #At file:///home/hakan/work/mysql/mysql-6.0-falcon-team/
>
> 2815 Hakan Kuecuekyilmaz 2008-09-09
> Rescued 3 tests from falcon_team test suite.
>
> Cleaned up test cases and added one -big.test version of
> falcon_bug_22207.test.
> added:
> mysql-test/suite/falcon/r/falcon_bug_22207-big.result
> mysql-test/suite/falcon/t/falcon_bug_22207-big.test
> renamed:
> mysql-test/suite/falcon_team/r/falcon_bug_22189.result =>
> mysql-test/suite/falcon/r/falcon_bug_22189.result
> mysql-test/suite/falcon_team/r/falcon_bug_22207.result =>
> mysql-test/suite/falcon/r/falcon_bug_22207.result
> mysql-test/suite/falcon_team/r/falcon_bug_24024.result =>
> mysql-test/suite/falcon/r/falcon_bug_24024.result
> mysql-test/suite/falcon_team/t/falcon_bug_22189.test =>
> mysql-test/suite/falcon/t/falcon_bug_22189.test
> mysql-test/suite/falcon_team/t/falcon_bug_22207.test =>
> mysql-test/suite/falcon/t/falcon_bug_22207.test
> mysql-test/suite/falcon_team/t/falcon_bug_24024.test =>
> mysql-test/suite/falcon/t/falcon_bug_24024.test
> modified:
> mysql-test/suite/falcon/r/falcon_bug_22972.result
> mysql-test/suite/falcon/t/disabled.def
> mysql-test/suite/falcon/t/falcon_bug_22972.test
> mysql-test/suite/falcon_team/r/falcon_bug_23945.result
> mysql-test/suite/falcon_team/t/falcon_bug_23945.test
> mysql-test/suite/falcon/r/falcon_bug_22189.result
> mysql-test/suite/falcon/r/falcon_bug_22207.result
> mysql-test/suite/falcon/r/falcon_bug_24024.result
> mysql-test/suite/falcon/t/falcon_bug_22189.test
> mysql-test/suite/falcon/t/falcon_bug_22207.test
> mysql-test/suite/falcon/t/falcon_bug_24024.test
Ideally I would like to see some commit comments explaining (the
reasoning behind) the actual changes to each file, instead of just
"cleaned up test cases". This makes reviews and future archeological
investigations easier.
> === renamed file 'mysql-test/suite/falcon_team/r/falcon_bug_24024.result' =>
> 'mysql-test/suite/falcon/r/falcon_bug_24024.result'
> --- a/mysql-test/suite/falcon_team/r/falcon_bug_24024.result 2008-04-20 00:05:17
> +0000
> +++ b/mysql-test/suite/falcon/r/falcon_bug_24024.result 2008-09-09 08:30:18 +0000
> @@ -14,10 +16,13 @@ a b
> 1 NULL
> 7 NULL
> DROP TABLE t1;
> -!!! Error message has to be decided.
> -!!! Something like "Cannot drop locked table" or similar would be fine.
> -!!! But never the misleading ERROR 42S02: Unknown table 't1'
> -COMMIT;
> -# Switch to session default and disconnect session con1
> +ERROR 42S02: Unknown table 't1'
> +SHOW WARNINGS;
> +Level Code Message
> +Warning 178 Can't execute the given command because you have active locked tables or
> an active transaction
Although this change is OK, I don't really like the fact that a request
of never using the misleading "Unknown table" error message for this
issue is removed.
Bug#22972 (duplicates Bug#24024) was closed after the following was
added to the changelog:
"Falcon cannot drop a table for which there is a pending transaction,
but the error message for such attempts was misleading."
It is clear that the error message is still misleading, so in my opinion
the bug has not been completely fixed, although there is a descriptive
warning message.
However, according to the bug report the issue seems to be fixed from
the Falcon side, since the problem is that the server does not recognize
these types of errors properly ("Falcon can report that the moon is
green and this same error will occur.").
So I'm OK with this particular change to the test, but I wonder if there
is a bug reported against the server regarding this?
---
Please explain why you did the following change:
> --- a/mysql-test/suite/falcon_team/t/falcon_bug_22189.test 2008-04-28 11:21:52 +0000
> +++ b/mysql-test/suite/falcon/t/falcon_bug_22189.test 2008-09-09 08:30:18 +0000
> @@ -68,8 +68,10 @@ SELECT * FROM x1;
>
> --echo # Switch to connection conn1
> connection conn1;
> ---send UPDATE x1 SET x1 = 0, x2 = 5
> ---send INSERT INTO x1 VALUES (0,6)
> +--error ER_DUP_ENTRY
> +UPDATE x1 SET x1 = 0, x2 = 5;
> +--error ER_DUP_ENTRY
> +INSERT INTO x1 VALUES (0,6);
>
> --echo # Switch to connection default
> connection default;
> @@ -78,8 +80,6 @@ ROLLBACK;
> --echo # Switch to connection conn1
> connection conn1;
> --error ER_DUP_ENTRY
> ---reap
> ---error ER_DUP_ENTRY
> INSERT INTO x1 VALUES (0,6);
> --error ER_DUP_ENTRY
> INSERT INTO x1 VALUES (0,6);
Changes to falcon_bug_23945.[test|result] look good.
This test is still problematic, since it causes subsequent tests in the
test suite to fail ("1005: Can't create table 'test.t1' (errno: 156)").
Until that is fixed, we should at least add a note and/or echo statement
about that in the test case, or possibly even disable it.
However, those things can be done later in a separate changeset if so
desired.
> === modified file 'mysql-test/suite/falcon_team/t/falcon_bug_23945.test'
> --- a/mysql-test/suite/falcon_team/t/falcon_bug_23945.test 2008-04-20 00:05:17 +0000
> +++ b/mysql-test/suite/falcon_team/t/falcon_bug_23945.test 2008-09-09 08:30:18 +0000
> @@ -1,19 +1,25 @@
> +--source include/have_falcon.inc
> +
> #
> # Bug #23945: crash during drop table, AUTOCOMMIT=0, CREATE TABLE .. AS SELECT
> #
> --echo *** Bug #23945 ***
>
> -
> -# --source include/have_innodb.inc
> -# SET STORAGE_ENGINE = InnoDB;
> ---source include/have_falcon.inc
> -SET STORAGE_ENGINE = Falcon;
> +# ----------------------------------------------------- #
> +# --- Initialisation --- #
> +# ----------------------------------------------------- #
> +let $engine = 'Falcon';
> +eval SET @@storage_engine = $engine;
>
> --disable_warnings
> DROP TABLE IF EXISTS t1;
> --enable_warnings
>
> -SET AUTOCOMMIT = 1;
> +SET @@autocommit = 1;
> +
> +# ----------------------------------------------------- #
> +# --- Test --- #
> +# ----------------------------------------------------- #
> --error ER_DUP_ENTRY
> CREATE TABLE t1 (PRIMARY KEY (a)) SELECT 1 AS a UNION ALL SELECT 1;
> --error ER_NO_SUCH_TABLE
> @@ -21,7 +27,7 @@ SELECT * FROM t1;
> --error ER_BAD_TABLE_ERROR
> DROP TABLE t1;
>
> -SET AUTOCOMMIT = 0;
> +SET @@autocommit = 0;
> --error ER_DUP_ENTRY
> CREATE TABLE t1 (PRIMARY KEY (a)) SELECT 1 AS a UNION ALL SELECT 1;
> --error ER_NO_SUCH_TABLE
> @@ -29,3 +35,15 @@ SELECT * FROM t1;
> # Here comes a server crash.
> --error ER_BAD_TABLE_ERROR
> DROP TABLE t1;
> +
> +# ----------------------------------------------------- #
> +# --- Check --- #
> +# ----------------------------------------------------- #
> +# Check not applicable in this test.
> +#SELECT count(*) FROM t1;
> +
> +# ----------------------------------------------------- #
> +# --- Final cleanup --- #
> +# ----------------------------------------------------- #
> +# Final cleanup not applicable in this test.
> +#DROP TABLE t1;
>
>
The rest looked all right. The patched falcon suite passed 100% on my
Linux test host.
--
John
