Re: bzr commit into mysql-6.0-falcon-team branch (lars-erik.bjork:2899) Bug#40130

[Lars-Erik Bjørk]

See inline answers

On Nov 4, 2008, at 3:02 PM, Hakan Kuecuekyilmaz wrote:

> Lars-Erik,
>
> patch looks good. Please see inline for minor improvements and
> questions.
>
> On Di, 2008-11-04 at 13:25 +0000, Lars-Erik.Bjork@stripped wrote:
>> #At file:///home/lb200670/devel/mysql/nanna21/
>>
>> 2899 lars-erik.bjork@stripped	2008-11-04
>>      This is a commit for bug#40130 (Falcon date / time indexes  
>> broken)
>>
>>      The bug was actually fixed by the patch to bug 40112 (Thanks
>>      Kevin:) ). This commit only includes a regression test for the  
>> bug.
>>
>>      Two files are added, these are:
>>
>>      mysql-test/suite/falcon/t/falcon_bug_40130.test
>>      mysql-test/suite/falcon/r/falcon_bug_40130.result
>> added:
>>  mysql-test/suite/falcon/r/falcon_bug_40130.result
>>  mysql-test/suite/falcon/t/falcon_bug_40130.test
>>
> [cut]
>
>> === added file 'mysql-test/suite/falcon/t/falcon_bug_40130.test'
>> --- a/mysql-test/suite/falcon/t/falcon_bug_40130.test	1970-01-01  
>> 00:00:00 +0000
>> +++ b/mysql-test/suite/falcon/t/falcon_bug_40130.test	2008-11-04  
>> 13:24:47 +0000
>> @@ -0,0 +1,101 @@
>> +--source include/have_falcon.inc
>> +
>> +#
>> +# Bug #40130: Falcon date / time indexes broken
>> +#
>> +--echo *** Bug #40130 ***
>> +
>> +# ----------------------------------------------------- #
>> +# --- Initialisation                                --- #
>> +# ----------------------------------------------------- #
>> +let $engine = 'Falcon';
>> +eval SET @@storage_engine = $engine;
>> +
>> +--disable_warnings
>> +DROP TABLE IF EXISTS `table10`;
>> +--enable_warnings
>> +
>> +# ----------------------------------------------------- #
>> +# --- Test                                          --- #
>> +# ----------------------------------------------------- #
>> +
>> +CREATE TABLE `table10` (`time_key` time, key (`time_key` ));
>> +INSERT IGNORE INTO table10 VALUES ('23:43:55');
>> +INSERT IGNORE INTO table10 VALUES ('03:18:59');
>> +INSERT IGNORE INTO table10 VALUES ('05:05:23');
>> +INSERT IGNORE INTO table10 VALUES ('09:20:40');
>> +INSERT IGNORE INTO table10 VALUES ('22:32:50');
>> +INSERT IGNORE INTO table10 VALUES ('07:41:31');
>> +INSERT IGNORE INTO table10 VALUES ('10:52:13');
>> +INSERT IGNORE INTO table10 VALUES ('12:40:54');
>> +INSERT IGNORE INTO table10 VALUES ('10:33:25');
>> +INSERT IGNORE INTO table10 VALUES ('22:11:46');
>> +
>
> What is the idea of INSERT IGNORE here?
>

This is from Philips random query generator, that's why IGNORE is  
there :)

>> +SET AUTOCOMMIT=OFF;
>> +
>> +START TRANSACTION ; UPDATE `table10` SET `time_key` = '09:11:23'  
>> WHERE `time_key` < '16:23:56' ; INSERT INTO `table10` VALUES  
>> ( '20:25:18' ) ; COMMIT;
>> +START TRANSACTION ; UPDATE `table10` SET `time_key` = '10:33:25'  
>> WHERE `time_key` > '22:11:46' ; INSERT INTO `table10` VALUES  
>> ( '17:58:48' ) ; COMMIT;
>> +START TRANSACTION ; INSERT INTO `table10` VALUES ( '00:16:10' ) ;  
>> UPDATE `table10` SET `time_key` = '16:05:35' WHERE `time_key` >  
>> '16:57:24' ; COMMIT;
>
> Can you please use one line per statement? That way future merges get
> easier.
>
> START TRANSACTION;
> UPDATE `table10` SET `time_key` = '09:11:23' WHERE `time_key` <
> '16:23:56';
> INSERT INTO `table10` VALUES ( '20:25:18' );
> COMMIT;
>
> I see your point in easier readability, but I also see  
> maintainability.
> Think of a future change, which would involve a merge:
>
> Change A:
> - START TRANSACTION ; UPDATE `table10` SET `time_key` = '09:11:23'
> + START TRANSACTION; UPDATE `table10` SET `time_key` = '09:11:23'
>
> Change B:
> - START TRANSACTION ; UPDATE `table10` SET `time_key` = '09:11:23'
> + START TRANSACTION ; UPDATE `table10` SET `time_key` = '19:11:23'
>
> Now when we try to merge changes A and B, we would get conflict.  
> Having
> every statement on a single line avoids such a situation:
>
> Change A:
> - START TRANSACTION ;
> + START TRANSACTION;
>
> Change B:
> - UPDATE `table10` SET `time_key` = '09:11:23'
> + UPDATE `table10` SET `time_key` = '19:11:23'
>
> Brian Aker blogged about this a while ago in more detail:
> "Coding Habits, What is on my mind while compiles are happening..."
>    http://krow.livejournal.com/558188.html
>
> [cut]
>

Point taken, I will change to one statement per line, and commit

>
>> +SELECT * FROM `table10`;
>
> Would a SELECT count(*) FROM table10; do it, too?
>

Yet another thing from the generator

I will remove the back ticks

>
>> +START TRANSACTION ;UPDATE `table10` SET `time_key` = '00:28:04'  
>> WHERE `time_key` < '09:58:46' ; UPDATE `table10` SET `time_key` =  
>> '04:56:25' WHERE `time_key` > '09:26:08' ; COMMIT;
>> +SELECT * FROM `table10`;
>
> Same here: I would favor one statement per line.
>
> [cut]
>
> Best,
>
> Hakan
>
>
>
> -- 
> Hakan Küçükyılmaz, Senior Software Engineer DBTG/MySQL +49 160
> 98953296
> Sun Microsystems GmbH     Sonnenallee 1, DE-85551 Kirchheim- 
> Heimstetten
> Geschaeftsfuehrer:  Thomas Schroeder, Wolfang Engels, Dr. Roland  
> Boemer
> Vorsitz d. Aufs.rat.: Martin Haering   HRB MUC 161028     49.011,  
> 8.376
>
>
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1
>