Hi Sergei,
On Monday 01 October 2007 17:29:10, Sergei Golubchik wrote:
>Hi!
>
>On Sep 10, Alexey Kopytov wrote:
>> >> --- 1.656/sql/mysqld.cc 2007-06-17 19:20:16 +04:00
>> >> +++ 1.657/sql/mysqld.cc 2007-07-10 17:37:22 +04:00
>> >> @@ -478,6 +478,40 @@ ulong expire_logs_days = 0;
>> >> ulong rpl_recovery_rank=0;
>> >> const char *log_output_str= "TABLE";
>> >>
>> >> +const double log_10[] = {
>> >> + 1e000, 1e001, 1e002, 1e003, 1e004, 1e005, 1e006, 1e007, 1e008,
>> >> 1e009, + 1e010, 1e011, 1e012, 1e013, 1e014, 1e015, 1e016, 1e017,
>> >> 1e018, 1e019, + 1e020, 1e021, 1e022, 1e023, 1e024, 1e025, 1e026,
>> >> 1e027, 1e028, 1e029, + 1e030, 1e031, 1e032, 1e033, 1e034, 1e035,
>> >> 1e036, 1e037, 1e038, 1e039, + 1e040, 1e041, 1e042, 1e043, 1e044,
>> >> 1e045, 1e046, 1e047, 1e048, 1e049, + 1e050, 1e051, 1e052, 1e053,
>> >> 1e054, 1e055, 1e056, 1e057, 1e058, 1e059, + 1e060, 1e061, 1e062,
>> >> 1e063, 1e064, 1e065, 1e066, 1e067, 1e068, 1e069, + 1e070, 1e071,
>> >> 1e072, 1e073, 1e074, 1e075, 1e076, 1e077, 1e078, 1e079, + 1e080,
>> >> 1e081, 1e082, 1e083, 1e084, 1e085, 1e086, 1e087, 1e088, 1e089, +
>> >> 1e090, 1e091, 1e092, 1e093, 1e094, 1e095, 1e096, 1e097, 1e098, 1e099, +
>> >> 1e100, 1e101, 1e102, 1e103, 1e104, 1e105, 1e106, 1e107, 1e108, 1e109,
>> >> + 1e110, 1e111, 1e112, 1e113, 1e114, 1e115, 1e116, 1e117, 1e118,
>> >> 1e119, + 1e120, 1e121, 1e122, 1e123, 1e124, 1e125, 1e126, 1e127,
>> >> 1e128, 1e129, + 1e130, 1e131, 1e132, 1e133, 1e134, 1e135, 1e136,
>> >> 1e137, 1e138, 1e139, + 1e140, 1e141, 1e142, 1e143, 1e144, 1e145,
>> >> 1e146, 1e147, 1e148, 1e149, + 1e150, 1e151, 1e152, 1e153, 1e154,
>> >> 1e155, 1e156, 1e157, 1e158, 1e159, + 1e160, 1e161, 1e162, 1e163,
>> >> 1e164, 1e165, 1e166, 1e167, 1e168, 1e169, + 1e170, 1e171, 1e172,
>> >> 1e173, 1e174, 1e175, 1e176, 1e177, 1e178, 1e179, + 1e180, 1e181,
>> >> 1e182, 1e183, 1e184, 1e185, 1e186, 1e187, 1e188, 1e189, + 1e190,
>> >> 1e191, 1e192, 1e193, 1e194, 1e195, 1e196, 1e197, 1e198, 1e199, +
>> >> 1e200, 1e201, 1e202, 1e203, 1e204, 1e205, 1e206, 1e207, 1e208, 1e209, +
>> >> 1e210, 1e211, 1e212, 1e213, 1e214, 1e215, 1e216, 1e217, 1e218, 1e219,
>> >> + 1e220, 1e221, 1e222, 1e223, 1e224, 1e225, 1e226, 1e227, 1e228,
>> >> 1e229, + 1e230, 1e231, 1e232, 1e233, 1e234, 1e235, 1e236, 1e237,
>> >> 1e238, 1e239, + 1e240, 1e241, 1e242, 1e243, 1e244, 1e245, 1e246,
>> >> 1e247, 1e248, 1e249, + 1e250, 1e251, 1e252, 1e253, 1e254, 1e255,
>> >> 1e256, 1e257, 1e258, 1e259, + 1e260, 1e261, 1e262, 1e263, 1e264,
>> >> 1e265, 1e266, 1e267, 1e268, 1e269, + 1e270, 1e271, 1e272, 1e273,
>> >> 1e274, 1e275, 1e276, 1e277, 1e278, 1e279, + 1e280, 1e281, 1e282,
>> >> 1e283, 1e284, 1e285, 1e286, 1e287, 1e288, 1e289, + 1e290, 1e291,
>> >> 1e292, 1e293, 1e294, 1e295, 1e296, 1e297, 1e298, 1e299, + 1e300,
>> >> 1e301, 1e302, 1e303, 1e304, 1e305, 1e306, 1e307, 1e308 +};
>> >
>> >It doesn't need to be that big now, does it ?
>> >perhaps we can get rid of it completely ?
>>
>> AFAIK, there is still one place where we need it, Field_real::truncate().
>> To be exact, the more powers of 10 we have here, the more chances to avoid
>> the "tight loop" here:
>>
>> uint order= field_length - dec;
>> uint step= array_elements(log_10) - 1;
>> max_value= 1.0;
>> for (; order > step; order-= step)
>> max_value*= log_10[step];
>> max_value*= log_10[order];
>> max_value-= 1.0 / log_10[dec];
>
>Yes, so I'd limit the array size to about 20 elements.
>I don't see a need to put this static array in every binary to optimize
>for a case of a DOUBLE(200,30) column (large field_length, that is).
>
I tried reducing the array size, and it doesn't work. We lose the precision
when calculating the max_value in Field_real::truncate() which results in the
following artifact:
-------------------------------------------------------
*** r/type_float.result Mon Sep 10 15:34:30 2007
--- r/type_float.reject Wed Nov 28 19:47:47 2007
***************
*** 347,352 ****
--- 347,355 ----
create table t1 (f1 double(200, 0));
insert into t1 values (1e199), (-1e199);
insert into t1 values (1e200), (-1e200);
+ Warnings:
+ Warning 1264 Out of range value for column 'f1' at row 1
+ Warning 1264 Out of range value for column 'f1' at row 2
insert into t1 values (2e200), (-2e200);
Warnings:
Warning 1264 Out of range value for column 'f1' at row 1
***************
*** 355,364 ****
f1 + 0e0
1e199
-1e199
! 1e200
! -1e200
! 1e200
! -1e200
drop table t1;
create table t1 (f1 float(30, 0));
insert into t1 values (1e29), (-1e29);
--- 358,367 ----
f1 + 0e0
1e199
-1e199
! 9.999999999999998e199
! -9.999999999999998e199
! 9.999999999999998e199
! -9.999999999999998e199
drop table t1;
create table t1 (f1 float(30, 0));
insert into t1 values (1e29), (-1e29);
-------------------------------------------------------
What's happening is that max_value is calculated imprecisely with
multiplications which results in the above test failure.
We also get performance degradation in Field_real::truncate() and
my_double_round() with this change.
Anyway, defining the above array increases the binary size by 309 * 8 = 2472
bytes, which does not look significant to me.
>> >> --- 1.65/mysql-test/r/func_group.result 2007-05-29 16:57:16 +04:00
>> >> +++ 1.66/mysql-test/r/func_group.result 2007-07-10 17:37:18 +04:00
>> >> @@ -891,7 +891,7 @@ select 1e8 * sum(distinct df) from t1;
>> >> 330000000
>> >> select 1e8 * min(df) from t1;
>> >> 1e8 * min(df)
>> >> -110000000
>> >> +110000000.00000001
>> >
>> >explain this change, please
>>
>> Two things:
>>
>> 1. Our decimal2double() conversion is imprecise due to 'naive' algorithm.
>> It would be better to just print the digits to a string buffer and then
>> run my_strtod() on it. Please let me know if you think this must be
>> included into the patch for WL#2934.
>
>Yes, I do.
>decimal2double() match the behaviour double2decimal().
>
Done it the new patch, though it didn't solve this particular problem :( The
only reason for this change is the second one that I mentioned earlier:
>2. As a consequence of the above, we get 1.1000000000000001 when converting
> a DECIMAL '1.1' to double. But String::set_real() previously involved
> sprintf("%14g") for conversion, so it was just hiding the computational
> error introduced by decimal2double().
Multiplication of 1e8 by 1.1 does result in 110000000.00000001 due to
computational error, it was just sprintf() which was hiding the fact
previously.
Although changing decimal2double() to use my_strtod() was not useless, see
fixes in sp.result in the new patch (we do have more precises results from
it).
>> >> --- 1.34/mysql-test/r/insert.result 2007-05-31 13:19:10 +04:00
>> >> +++ 1.35/mysql-test/r/insert.result 2007-07-10 17:37:19 +04:00
>> >> @@ -159,6 +159,7 @@ set @value= "1e+1111111111a";
>> >> insert into t1
>> >> values(null,@value,@value,@value,@value,@value,@value,@value,@value,@va
>> >>lue ); Warnings:
>> >> Warning 1264 Out of range value for column 'f_double' at row 1
>> >> +Warning 1264 Out of range value for column 'f_double' at row 1
>> >
>> >why a duplicate warning ?
>>
>> We get one from my_strtod() when it truncates "1e+1111111111a" to +Inf,
>> and another from if Field_real::truncate() which truncates +Inf to
>> DBL_MAX.
>
>Why not to truncate to DBL_MAX in my_strtod() ?
Done in the new patch.
>Or, alternatively, not to issue a warning when truncating to Inf, as Inf
>will cause a warning later anyway.
>
>> >> --- 1.61/mysql-test/r/ps_2myisam.result 2007-06-05 19:51:44 +04:00
>> >> +++ 1.62/mysql-test/r/ps_2myisam.result 2007-07-10 17:37:19 +04:00
>> >> @@ -2822,10 +2822,10 @@ c1 c20 c21 c22 c23 c24 c25 c26 c27 c28 c
>> >> 51 5 51.0 51.0 51.0 51.0 51.0 51.0 51.0 51.0 51.0 51.0
>> >> 52 5 52.0 52.0 52.0 52.0 52.0 52.0 52.0 52.0 52.0 52.0
>> >> 53 5 53.0 53.0 53.0 53.0 53.0 53.0 53.0 53.0 53.0 53.0
>> >> -54 5 54 54 54.00 54.00 54.00 54.00 54.00 54.00 54.00 54.00
>> >> +54 0 54 54 54.00 54.00 54.00 54.00 54.00 54.00 54.00 54.00
>> >
>> >This is probably a bad idea.
>> >when you cannot convert a number to a string in a too small buffer
>> >you return 0. Old code was truncating the string, which I believe was
>> >correct. It's like assigning a long string to a short buffer -
>> >a string should be truncated.
>>
>> It looks like a "gray area" to me. The counter-argument would be that when
>> truncating a string the result is always a valid string. When truncating
>> string representations of a number, the result may not even be a valid
>> string representation of a number.
>
>Yes, but still - first you convert, then truncate.
>That's the old behaviour, at least.
>
Done in the new patch.
>> >> +INSERT INTO d1 VALUES (1.79769313486232e+308);
>> >> +SELECT * FROM d1;
>> >> +DROP TABLE d1;
>> >> +
>> >> --- 1.59/mysql-test/r/type_newdecimal.result 2007-05-21 21:31:05 +04:00
>> >> +++ 1.60/mysql-test/r/type_newdecimal.result 2007-07-10 17:37:20 +04:00
>> >> @@ -1175,21 +1167,21 @@ CAST(my_float AS DECIMAL(65,30)) my_fl
>> >> 0.000000000000000001175494374380 1.17549e-18
>> >> 0.000000000000000011754943743802 1.17549e-17
>> >> 0.000000000000000117549432474939 1.17549e-16
>> >> -0.000000000000001175494324749389 1.17549e-15
>> >> -0.000000000000011754943671010360 1.17549e-14
>> >> -0.000000000000117549429933840000 1.17549e-13
>> >> -0.000000000001175494380653563000 1.17549e-12
>> >> -0.000000000011754943372854760000 1.17549e-11
>> >> -0.000000000117549428524377200000 1.17549e-10
>> >> -0.000000001175494368510499000000 1.17549e-09
>> >> -0.000000011754943685104990000000 1.17549e-08
>> >> -0.000000117549433298336200000000 1.17549e-07
>> >> -0.000001175494389826781000000000 1.17549e-06
>> >> -0.000011754943443520460000000000 1.17549e-05
>> >> +0.000000000000001175494324749389 0.00000000000000117549
>> >> +0.000000000000011754943671010362 0.0000000000000117549
>> >> +0.000000000000117549429933840040 0.000000000000117549
>> >> +0.000000000001175494380653563400 0.00000000000117549
>> >> +0.000000000011754943372854765000 0.0000000000117549
>> >> +0.000000000117549428524377220000 0.000000000117549
>> >> +0.000000001175494368510499000000 0.00000000117549
>> >> +0.000000011754943685104990000000 0.0000000117549
>> >> +0.000000117549433298336230000000 0.000000117549
>> >> +0.000001175494389826781100000000 0.00000117549
>> >> +0.000011754943443520460000000000 0.0000117549
>> >> 0.000117549432616215200000000000 0.000117549
>> >> -0.001175494398921728000000000000 0.00117549
>> >> +0.001175494398921728100000000000 0.00117549
>> >> -0.011754943057894710000000000000 0.0117549
>> >> +0.011754943057894707000000000000 0.0117549
>> >> -0.117549434304237400000000000000 0.117549
>> >> +0.117549434304237370000000000000 0.117549
>> >
>> >Hm. Here we cast a *float* to a decimal. From the results
>> >it's clear that dtoa uses a double precision here.
>> >Could you fix it so that it would use correct precision ?
>>
>> This is what WL#3977 is about. I have no idea how to fix this
>> "locally". We need float2decimal(), and then we need
>> Field::val_float() and so on.
>
>Ok. Please add a note to the test case (in type_newdecimal.test) that
>the result is not good (I avoid to use the word "wrong" here :), and
>improving it is the subject of WL#3977.
>
Done in the new patch.
Best regards,
--
Alexey Kopytov, Software Developer
MySQL AB, www.mysql.com
Are you MySQL certified? www.mysql.com/certification
