Re: [Maria-developers] [Commits] Rev 4358: MDEV-7113 difference between check_vcol_func_processor and check_partition_func_processor in lp:~maria-captains/maria/5.5
Hi Sergei, * what about persistent virtual columns? Do we still support environment-dependent virtual functions for those? * the behavior of current solution is not deterministic: thread1> select @@time_zone; +-------------+ | @@time_zone | +-------------+ | SYSTEM | +-------------+ 1 row in set (0.00 sec) thread1> create table t2 ( a varchar(32), b int, v varchar(100) as (from_unixtime(b))); Query OK, 0 rows affected (0.28 sec) thread1> insert into t2 (a,b) values ('local-tz-23-11', unix_timestamp()); Query OK, 1 row affected (0.04 sec) thread1> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec) thread2> set time_zone='Europe/Helsinki'; Query OK, 0 rows affected (0.00 sec) thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec) thread2> flush tables; Query OK, 0 rows affected (0.00 sec) thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 22:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec) Is this really ok? Maybe, it makes sense to use some some "default" (e.g. global variables) environment on the non-deterministic virtual column functions? Or, make these functions always fetch the environment from the current thread? On Mon, Nov 17, 2014 at 09:43:46AM +0100, Sergei Golubchik wrote:
At lp:~maria-captains/maria/5.5
------------------------------------------------------------ revno: 4358 revision-id: sergii@pisem.net-20141115155753-rx5twf6utu1isczh parent: sergii@pisem.net-20141113173729-hirl67og6at3xbni fixes bugs: https://mariadb.atlassian.net/browse/MDEV-6789 https://mariadb.atlassian.net/browse/MDEV-7113 committer: Sergei Golubchik <sergii@pisem.net> branch nick: 5.5 timestamp: Sat 2014-11-15 16:57:53 +0100 message: MDEV-7113 difference between check_vcol_func_processor and check_partition_func_processor MDEV-6789 segfault in Item_func_from_unixtime::get_date on updating table with virtual columns
* prohibit VALUES in partitioning expression * prohibit user and system variables in virtual column expressions * fix Item_func_date_format to cache locale (for %M/%W to return the same as MONTHNAME/DAYNAME) * fix Item_func_from_unixtime to cache time_zone directly, not THD (and not to crash) * added tests for other incorrectly allowed (in vcols) functions to see that they don't crash ...
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
Hi, Sergey! On Nov 19, Sergey Petrunia wrote:
Hi Sergei,
* what about persistent virtual columns? Do we still support environment-dependent virtual functions for those?
I've explained that in a comment - I was mostly trying to fix crashes, like that one with FROM_UNIXTIME. I'm hesitant to reduce the set of supported functions in GA version. I think we can do it in 10.1 or 10.2.
* the behavior of current solution is not deterministic:
thread1> select @@time_zone; +-------------+ | @@time_zone | +-------------+ | SYSTEM | +-------------+ 1 row in set (0.00 sec)
thread1> create table t2 ( a varchar(32), b int, v varchar(100) as (from_unixtime(b))); Query OK, 0 rows affected (0.28 sec)
thread1> insert into t2 (a,b) values ('local-tz-23-11', unix_timestamp()); Query OK, 1 row affected (0.04 sec)
thread1> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
thread2> set time_zone='Europe/Helsinki'; Query OK, 0 rows affected (0.00 sec)
thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
thread2> flush tables; Query OK, 0 rows affected (0.00 sec)
thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 22:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
Yes, I have that in the test. For FROM_UNIXTIME, for division, and for MONTHNAME, DAYNAME and DATE_FORMAT.
Is this really ok? Maybe, it makes sense to use some some "default" (e.g. global variables) environment on the non-deterministic virtual column functions?
Not that's not really ok, but... see above. I've only changed FROM_UNIXTIME not to crash (using the same approach that MONTHNAME and DAYNAME were already using), and fixed DATE_FORMAT to return the same as MONTHNAME and DAYNAME. I don't like the current behavior and I've shown that it's buggy in the test. But I don't think we can change it in 5.5 or 10.0
Or, make these functions always fetch the environment from the current thread?
Calling current_thd from val* methods? Regards, Sergei
Hi Sergei, I think, the below replies have addressed all my concerns. For its limited purpose, the patch is ok and can be pushed. I trust there are already other MDEVs to fix the known issues that are not fixed by this patch? On Wed, Nov 19, 2014 at 09:36:47PM +0100, Sergei Golubchik wrote:
On Nov 19, Sergey Petrunia wrote:
* what about persistent virtual columns? Do we still support environment-dependent virtual functions for those?
I've explained that in a comment - I was mostly trying to fix crashes, like that one with FROM_UNIXTIME. I'm hesitant to reduce the set of supported functions in GA version. I think we can do it in 10.1 or 10.2.
* the behavior of current solution is not deterministic:
thread1> select @@time_zone; +-------------+ | @@time_zone | +-------------+ | SYSTEM | +-------------+ 1 row in set (0.00 sec)
thread1> create table t2 ( a varchar(32), b int, v varchar(100) as (from_unixtime(b))); Query OK, 0 rows affected (0.28 sec)
thread1> insert into t2 (a,b) values ('local-tz-23-11', unix_timestamp()); Query OK, 1 row affected (0.04 sec)
thread1> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
thread2> set time_zone='Europe/Helsinki'; Query OK, 0 rows affected (0.00 sec)
thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 23:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
thread2> flush tables; Query OK, 0 rows affected (0.00 sec)
thread2> select * from t2; +----------------+------------+---------------------+ | a | b | v | +----------------+------------+---------------------+ | local-tz-23-11 | 1416427891 | 2014-11-19 22:11:31 | +----------------+------------+---------------------+ 1 row in set (0.00 sec)
Yes, I have that in the test. For FROM_UNIXTIME, for division, and for MONTHNAME, DAYNAME and DATE_FORMAT.
Is this really ok? Maybe, it makes sense to use some some "default" (e.g. global variables) environment on the non-deterministic virtual column functions?
Not that's not really ok, but... see above. I've only changed FROM_UNIXTIME not to crash (using the same approach that MONTHNAME and DAYNAME were already using), and fixed DATE_FORMAT to return the same as MONTHNAME and DAYNAME.
I don't like the current behavior and I've shown that it's buggy in the test. But I don't think we can change it in 5.5 or 10.0
Or, make these functions always fetch the environment from the current thread?
Calling current_thd from val* methods?
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
participants (2)
-
Sergei Golubchik -
Sergey Petrunia