Hi, Jacob! Even if MDEV is not "In Review", here're few early thoughts to consider: On Mar 20, jacob.mathew@mariadb.com wrote:
revision-id: 06dac6f23f662d6892fddd47c4566c96a3d044d3 (mariadb-10.2.3-180-g06dac6f23f6) parent(s): cd53525d7c97a7c2b69873fbbd28982844e3cae3 author: Jacob Mathew committer: Jacob Mathew timestamp: 2017-03-20 16:35:31 -0700 message:
MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT
Fixed handling of default values with BETWEEN and temporal functions. Added test case.
diff --git a/mysql-test/r/func_default_between_temporal.result b/mysql-test/r/func_default_between_temporal.result new file mode 100644 index 00000000000..a1d11f56d4d --- /dev/null +++ b/mysql-test/r/func_default_between_temporal.result @@ -0,0 +1,11 @@ +CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) ); +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `col` int(11) DEFAULT (1 like (current_timestamp() between 2000-01-01 00:00:00 and 2012-12-12 00:00:00))
Note, that the generated create table statement is invalid. Temporal constants are printed unquoted.
+) ENGINE=MyISAM DEFAULT CHARSET=latin1 +INSERT INTO t1 VALUES( DEFAULT ); +SELECT * FROM t1; +col +0 +DROP TABLE t1; diff --git a/sql/item.cc b/sql/item.cc index dd2c4fd75a5..5c0afc526f8 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -9378,17 +9378,30 @@ void Item_cache::store(Item *item)
void Item_cache::print(String *str, enum_query_type query_type) { - if (value_cached) - { - print_value(str); - return; - } - str->append(STRING_WITH_LEN("<cache>(")); - if (example) - example->print(str, query_type); - else - Item::print(str, query_type); - str->append(')'); + if ( ( query_type & QT_ITEM_ORIGINAL_FUNC_NULLIF ) && + example && ( example->type() == FUNC_ITEM ) ) + { + // Instead of "cache" or the cached value, print the function name, + // which is more meaningful to the end user + const char *func_name = ( ( Item_func* ) example )->func_name(); + + str->append( func_name ); + str->append( "()" );
1. Why a check for QT_ITEM_ORIGINAL_FUNC_NULLIF? Is it how you distinguish SHOW CREATE from EXPLAIN? 2. Why do you do that explicitly, instead of example->print()? The item knows how to print itself. You don't. For example, the function can have arguments, not "()", or may be it needs to be printed using infix notation (like Item_func_plus, and some others).
+ } + else + { + if (value_cached) + { + print_value(str); + return; + } + str->append(STRING_WITH_LEN("<cache>(")); + if (example) + example->print(str, query_type); + else + Item::print(str, query_type); + str->append(')'); + }
Regards, Sergei Chief Architect MariaDB and security@mariadb.org