Hema,
Patch not approved. I have comments.
1) The patch is malformed and includes an extra bit at the top which should
not be there. .bzr-mysql...
2) The tests will not run.
Unexpected line ' ' found in
'd:/source/bzr/mysql-6.0-wl-4222/mysql-test/suite/b
ackup/combinations' at lib//My/Config.pm line 203, <GEN12> line 3.
This was due to extra characters in the file. I changed it to:
--- a/mysql-test/suite/backup/combinations 1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/combinations 2008-06-05 18:18:32 +0000
@@ -0,0 +1,12 @@
+[innodb]
+--default-storage-engine=innodb
+[falcon]
+--default-storage-engine=falcon
+[myisam]
+--default-storage-engine=myisam
+[memory]
+--default-storage-engine=memory
+
3) Why are these tests in a backup suite but the rest are not? Is there some
other patch that moves the backup tests to the backup suite? Seems silly to
have some in one place and others in another.
4) I don't like the names of some of the tests. The names start out
meaningful but the naming scheme is inconsistent. For example, shouldn't
'backup_objectsfall' be 'backup_functions'? That would seem to make more
sense. When I first saw the test name I though it was a test that is only
run in the Fall...as opposed to Spring, Summer.... :P Same goes for the
other two tests... backup_procedures, backup_triggers.
5) You need to include the 'must have' list to include falcon, innodb in all
of your tests. Not every build will build these engines -- especially
falcon.
6) Why are all of the statements on one line? It would make for easier
reading if, for example, CREATE were written like this:
CREATE TABLE teams (
teamno int not null primary key,
playerno int,
division char(10),
foreign key(playerno)
references players(playerno)
);
...you get the idea.
7) It has been requested of me in the past by QA folk that there is a
standard for case usage. For example, all SQL command keywords are uppercase
while identifiers are lowercase. Above should be:
CREATE TABLE teams (
teamno INT NOT NULL PRIMARY KEY,
playerno INT,
division CHAR(10),
FOREIGN KEY(playerno)
REFERENCES players(playerno)
);
...please check with Jeb to confirm this.
8) It has also been suggested to me that we should avoid using 'USE db' and
prefix all statements with the db name. SELECT * FROM db.t1; ...please check
with Jeb to confirm this.
9) There are spacing issues. For example:
+ INSERT INTO matches
values(6,1,3,3,1),(7,2,1,4,2),(8,1,5,0,1),(9,2,2,3,0),(10,2,2,2,2) ;
^
^
10) Watch line breaks. This should be formatted so that words are broken
across lines at 'legal' points:
+# Purpose: To test the metadata consistency by using object trigger for all
sto
+# rage engines
What's a sto rage engine? Is it a really angry sto?
11) Do you really need to mask so many columns?
+--replace_column 2 # 3 # 4 # 5 # 6 # 7 # 8 # 9 # 10 # 11 # 12 # 13 # 14 #
16 # 17 # 18 #
+SHOW TABLE STATUS;
If so, then perhaps there is a different command you can use...
12) Once the patch and combinations file was fixed, I got this error:
backup.backup_objectsfall 'innodb' [ fail ]
mysqltest: At line 9: Could not open
'.\data2\bzr6.04june\mysql-6.0-backup\mysql
-test\suite\backup\include\backup_engine.inc' for reading
Which is caused by a hard-coded path in your test file:
--source
/data2/bzr6.04june/mysql-6.0-backup/mysql-test/suite/backup/include/backup_e
ngine.inc
The above should be:
--source suite/backup/include/backup_engine.inc
13) Once the patch issue was resolved, the result file was different but
this was due to the patch problem. The same is true for the other two tests.
14) Once all those problems were solved, I got this. I surrender. Please fix
and resubmit a new patch.
backup.backup_objectts 'innodb' [ fail ]
mysqltest: At line 181: query 'RESTORE FROM 'bup_objectts.bak'' failed: 7:
Error
on rename of '.\bup_objectts\cap.TRG~' to '.\bup_objectts\cap.TRG'
(Errcode: 17
)
The result from queries just before the failure was:
< snip >
Table t2
Create Table CREATE TABLE `t2` (
`a` char(4) DEFAULT NULL
) ENGINE=ENGINE DEFAULT CHARSET=latin1
SHOW TABLE STATUS;
Name Engine Version Row_format Rows Avg_row_length
Data_lengthMax_d
ata_length Index_length Data_free Auto_increment
Create_timeUpdat
e_time Check_time Collation Checksum Create_options
Comment
cap # # # # # # # #
#####lat
in1_swedish_ci # # #
city # # # # # # # #
#####lat
in1_swedish_ci # # #
dropcity # # # # # # #
######la
tin1_swedish_ci # # #
t2 # # # # # # # #
#####lat
in1_swedish_ci # # #
t3 # # # # # # # #
#####lat
in1_swedish_ci # # #
t4 # # # # # # # #
#####lat
in1_swedish_ci # # #
backup data
BACKUP DATABASE bup_objectts TO 'bup_objectts.bak';
backup_id
#
dropping database.
DROP DATABASE bup_objectts;
RESTORE FROM bup_objectts.bak;
RESTORE FROM 'bup_objectts.bak';
Chuck
