Dmitri, hello.
Thanks for discussions and review.
I fixed all items you pointed out.
cheers,
Andrei
> Hello Andrei!
>
>
> Here is my review for your patch:
>
> * Andrei Elkin <aelkin@stripped> [07/09/14 19:59]:
>> ChangeSet@stripped, 2007-09-14 17:01:53+03:00, aelkin@stripped +3 -0
>> Bug #26199 Replication Failure on Slave when using stored procs with bit-type
> parameters.
>>
>> The value of the actual argument of BIT-type-arg stored procedure was binlogged
> as non-espaped sequence of
>> bytes corresponding to internal representation of the bit value.
>
> non-escaped :)
>
> Also in future please keep lines in ChangeSet comment shorter than 80
> characters to make it more convinient for e-mail quoting.
>
>> The patch enforces binlogging of the bit-argument as a valid literal: prefixing
> the quoted bytes sequence with
>> _binary.
>
> I think it makes sense to mention here and below that this value is also
> escaped if it is needed (and not only quoted).
>
> ...
>
>> sql/sp_head.cc@stripped, 2007-09-14 17:01:43+03:00, aelkin@stripped +3 -2
>> Treating BIT field type specially to for its value to be prefixed and
> quoted.
>
> ...
>
>> diff -Nrup a/mysql-test/t/rpl_sp_effects.test b/mysql-test/t/rpl_sp_effects.test
>> --- a/mysql-test/t/rpl_sp_effects.test 2005-09-07 18:39:41 +03:00
>> +++ b/mysql-test/t/rpl_sp_effects.test 2007-09-14 17:01:43 +03:00
>> @@ -195,9 +195,44 @@ sync_slave_with_master;
>> connection slave;
>> select 'slave', a from t1;
>>
>> +# bug#26199 Replication Failure on Slave when using stored procs with bit-type
> parameters
>> +
>> connection master;
>> -drop table t1;
>> +
>> +create table t2 (b BIT(7));
>> +delimiter //;
>> +create procedure sp_bug26199(bitvalue BIT(7))
>> +begin
>> + insert into t2 set b = bitvalue;
>> +end //
>> +
>> +create function sf_bug26199(b BIT(7)) returns int
>> +begin
>> + insert into t2 values(b);
>> + return 0;
>> +end//
>
> I think it is better not to mix this test with cleanup section for the
> previous test case. Also IMO it makes sense to stick to the style which
> is used in other tests where we use bug-specific table/routine names and
> try to drop objects before creating them (e.g. see sp.test).
>
>> +
>> +DELIMITER ;//
>> +
>> +
>> +
>> +call sp_bug26199(b'1110');
>> +select sf_bug26199(b'1111111');
>> +select sf_bug26199(b'101111111');
>> +select hex(b) from t2;
>
> How about testing value which will require escaping in addition
> to the quoting (like '\0' or '\'') ?
>
>> +
>> +sync_slave_with_master;
>> +#connection slave;
>> +select hex(b) from t2;
>> +
>> +
>> +
>> +
>> +connection master;
>> +drop table t1,t2;
>> drop function f1;
>> drop function f2;
>> drop procedure p1;
>> +drop procedure sp_bug26199;
>> +drop function sf_bug26199;
>> sync_slave_with_master;
>> diff -Nrup a/sql/sp_head.cc b/sql/sp_head.cc
>> --- a/sql/sp_head.cc 2007-07-31 15:23:23 +03:00
>> +++ b/sql/sp_head.cc 2007-09-14 17:01:43 +03:00
>> @@ -100,8 +100,9 @@ sp_get_item_value(THD *thd, Item *item,
>> case REAL_RESULT:
>> case INT_RESULT:
>> case DECIMAL_RESULT:
>> - return item->val_str(str);
>> -
>> + if (item->field_type() != MYSQL_TYPE_BIT)
>> + return item->val_str(str);
>> + else {/* Bit type is handled as binary string */}
>> case STRING_RESULT:
>> {
>> String *result= item->val_str(str);
>
> Hmm... in future we might need more generic way for handling of such
> 'strange' types, but such solution is probably OK at the moment.
>
> I think it is OK to push this patch after fixing/discussing minor
> issues mentioned above.
>
> --
> Dmitri Lenev, Software Developer
> MySQL AB, www.mysql.com
>
> Are you MySQL certified? http://www.mysql.com/certification
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>
