maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Jacob!

Even if MDEV is not "In Review", here're few early thoughts to consider:

On Mar 20, jacob.mathew@xxxxxxxxxxx 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@xxxxxxxxxxx