[MDEV-10355] Weird error message upon CREATE TABLE with DEFAULT: Function or expression 'cache' cannot be used in the ??? clause Created: 2016-07-10  Updated: 2021-12-28  Resolved: 2017-04-19

Status: Closed
Project: MariaDB Server
Component/s: OTHER
Affects Version/s: 10.2
Fix Version/s: 10.2.6

Type: Bug Priority: Major
Reporter: Elena Stepanova Assignee: Jacob Mathew (Inactive)
Resolution: Fixed Votes: 0
Labels: 10.2-ga

Issue Links:
Relates
relates to MDEV-10134 Add full support for DEFAULT Closed
relates to MDEV-25672 table alias from previous statement i... Closed
Sprint: 10.2.6-2, 10.2.6-3

 Description   

MariaDB [test]> CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) );
ERROR 1901 (HY000): Function or expression 'cache' cannot be used in the ??? clause of `col`



 Comments   
Comment by Jacob Mathew (Inactive) [ 2017-02-08 ]

In 10.1, the parser returns a syntax error for this statement:
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) )' at line 1

In 10.2, the parser does not return a syntax error. During statement execution, the weird error message is returned.

Comment by Elena Stepanova [ 2017-02-09 ]

jacob-mathew,
Naturally it returns a syntax error in 10.1 and does not in 10.2, because it's a new feature – in 10.2 the syntax was extended in scope of the linked task MDEV-10134.

Comment by Jacob Mathew (Inactive) [ 2017-02-09 ]

In 10.2, the call for generating the error message during execution of the statement is:
my_error(ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), res.name,
"?", "???");

That is the reason for the question marks in the error message. Those placeholders were never filled in. Also the function name 'cache', which is the value of res.name, needs to be fixed. Fixing the parameters in this call will fix the problem.

Comment by Jacob Mathew (Inactive) [ 2017-02-09 ]

I have so far made the changes to replace the question marks.

Comment by Jacob Mathew (Inactive) [ 2017-02-14 ]

I have completed my changes to fix the bug. The changes are ready to be reviewed.

Comment by Jacob Mathew (Inactive) [ 2017-02-14 ]

Hi Sergei,

Please review my changes to fix CREATE TABLE bug MDEV-10355.

Thanks,
Jacob

commit cd53525d7c97a7c2b69873fbbd28982844e3cae3
Author: Jacob Mathew <jacob.mathew@mariadb.com>
Date:   Fri Feb 10 13:41:34 2017 -0800
 
    Changes to fix CREATE TABLE bug MDEV_10355.
 
diff --git a/sql/item.cc b/sql/item.cc
index 369b2df2675..dd2c4fd75a5 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -9478,7 +9478,7 @@ Item *Item_cache_int::convert_to_basic_const_item(THD *thd)
 
 Item_cache_temporal::Item_cache_temporal(THD *thd,
                                          enum_field_types field_type_arg):
-  Item_cache_int(thd, field_type_arg)
+  Item_cache_int(thd, field_type_arg), cached_temporal_item_name(NULL)
 {
   if (mysql_type_to_time_type(Item_cache_temporal::field_type()) ==
                               MYSQL_TIMESTAMP_ERROR)
@@ -9614,11 +9614,29 @@ int Item_cache_temporal::save_in_field(Field *field, bool no_conversions)
 
 void Item_cache_temporal::store_packed(longlong val_arg, Item *example_arg)
 {
-  /* An explicit values is given, save it. */
+  /* An explicit value is given, save it. */
   store(example_arg);
   value_cached= true;
   value= val_arg;
   null_value= false;
+
+  switch (example_arg->type())
+  {
+    case FUNC_ITEM:
+    {
+      // Instead of "cache", use the actual function name,
+      // which is more meaningful to the end user
+      const char *func_name     = ( ( Item_func* ) example_arg )->func_name();
+      char       *ptr           = ( char* ) current_thd->alloc( strlen( func_name ) + 3 );
+
+      if ( ptr )
+      {
+        strxmov( ptr, func_name, "()", NullS );
+      }
+
+      cached_temporal_item_name = ptr;
+    }
+  }
 }
 
 
diff --git a/sql/item.h b/sql/item.h
index bb2f05b4c6d..72b48b5e421 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -5436,6 +5436,7 @@ class Item_cache: public Item_basic_constant,
 
   static Item_cache* get_cache(THD *thd, const Item *item);
   static Item_cache* get_cache(THD *thd, const Item* item, const Item_result type);
+  virtual const char* cached_item_name() const { return "cache"; }
   virtual void keep_array() {}
   virtual void print(String *str, enum_query_type query_type);
   bool eq_def(const Field *field) 
@@ -5448,7 +5449,7 @@ class Item_cache: public Item_basic_constant,
   }
   bool check_vcol_func_processor(void *arg) 
   {
-    return mark_unsupported_function("cache", arg, VCOL_IMPOSSIBLE);
+    return mark_unsupported_function(cached_item_name(), arg, VCOL_IMPOSSIBLE);
   }
   /**
      Check if saved item has a non-NULL value.
@@ -5529,6 +5530,7 @@ class Item_cache_int: public Item_cache
 
 class Item_cache_temporal: public Item_cache_int
 {
+  const char *cached_temporal_item_name;
 public:
   Item_cache_temporal(THD *thd, enum_field_types field_type_arg);
   String* val_str(String *str);
@@ -5551,6 +5553,8 @@ class Item_cache_temporal: public Item_cache_int
   Item *convert_to_basic_const_item(THD *thd);
   Item *get_copy(THD *thd, MEM_ROOT *mem_root)
   { return get_item_copy<Item_cache_temporal>(thd, mem_root, this); }
+  const char *cached_item_name() const
+  { return cached_temporal_item_name ? cached_temporal_item_name : "cache"; }
 };
 
 
diff --git a/sql/table.cc b/sql/table.cc
index 4242b870d55..10f5388e380 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -48,7 +48,7 @@
 #define MYSQL57_GCOL_HEADER_SIZE 4
 
 static Virtual_column_info * unpack_vcol_info_from_frm(THD *, MEM_ROOT *,
-              TABLE *, String *, Virtual_column_info **, bool *);
+              TABLE *, String *, Virtual_column_info **, uint, bool *);
 static bool check_vcol_forward_refs(Field *, Virtual_column_info *);
 
 /* INFORMATION_SCHEMA name */
@@ -1065,24 +1065,24 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
     case VCOL_GENERATED_VIRTUAL:
     case VCOL_GENERATED_STORED:
       vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
-                                    &((*field_ptr)->vcol_info), error_reported);
+                                    &((*field_ptr)->vcol_info), type, error_reported);
       *(vfield_ptr++)= *field_ptr;
       break;
     case VCOL_DEFAULT:
       vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
                                       &((*field_ptr)->default_value),
-                                      error_reported);
+                                      type, error_reported);
       *(dfield_ptr++)= *field_ptr;
       break;
     case VCOL_CHECK_FIELD:
       vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
                                       &((*field_ptr)->check_constraint),
-                                      error_reported);
+                                      type, error_reported);
       *check_constraint_ptr++= (*field_ptr)->check_constraint;
       break;
     case VCOL_CHECK_TABLE:
       vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
-                                      check_constraint_ptr, error_reported);
+                                      check_constraint_ptr, type, error_reported);
       check_constraint_ptr++;
       break;
     }
@@ -1103,7 +1103,7 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
       expr_str.append(')');
       vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
                                       &((*field_ptr)->default_value),
-                                      error_reported);
+                                      VCOL_DEFAULT, error_reported);
       *(dfield_ptr++)= *field_ptr;
       if (!field->default_value->expr)
         goto end;
@@ -2774,7 +2774,7 @@ bool fix_session_vcol_expr_for_read(THD *thd, Field *field,
 */
 
 static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
-                                    Virtual_column_info *vcol)
+                                    Virtual_column_info *vcol, uint vcol_type)
 {
   Item* func_expr= vcol->expr;
   DBUG_ENTER("fix_and_check_vcol_expr");
@@ -2809,9 +2809,14 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
 
   int error= func_expr->walk(&Item::check_vcol_func_processor, 0, &res);
   if (error || (res.errors & VCOL_IMPOSSIBLE))
-  { // this can only happen if the frm was corrupted
+  {
+    // This can legitimately occur if the column is virtual or has a
+    // default value that is cached.  When the statement is originally
+    // parsed, the parser needs to allow cached values in order to
+    // handle CREATE with a SELECT, so the parser would not have
+    // encountered this error.
     my_error(ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), res.name,
-             "???", "?????");
+             vcol_type_name((enum_vcol_info_type) vcol_type), vcol->name.str);
     DBUG_RETURN(1);
   }
   vcol->flags= res.errors;
@@ -2864,7 +2869,7 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
 static Virtual_column_info *
 unpack_vcol_info_from_frm(THD *thd, MEM_ROOT *mem_root, TABLE *table,
                           String *expr_str, Virtual_column_info **vcol_ptr,
-                          bool *error_reported)
+                          uint vcol_type, bool *error_reported)
 {
   Create_field vcol_storage; // placeholder for vcol_info
   Parser_state parser_state;
@@ -2892,7 +2897,7 @@ unpack_vcol_info_from_frm(THD *thd, MEM_ROOT *mem_root, TABLE *table,
   vcol_storage.vcol_info->stored_in_db=      vcol->stored_in_db;
   vcol_storage.vcol_info->name=              vcol->name;
   vcol_storage.vcol_info->utf8=              vcol->utf8;
-  if (!fix_and_check_vcol_expr(thd, table, vcol_storage.vcol_info))
+  if (!fix_and_check_vcol_expr(thd, table, vcol_storage.vcol_info, vcol_type))
   {
     *vcol_ptr= vcol_info= vcol_storage.vcol_info;   // Expression ok
     DBUG_ASSERT(vcol_info->expr);

Comment by Jacob Mathew (Inactive) [ 2017-03-29 ]

In this commit, I have corrected the original problem and some others that I discovered afterwards:

https://github.com/MariaDB/server/commit/b4a223e338152c8764c980aeae59fb927eea1b0b

Comment by Jacob Mathew (Inactive) [ 2017-03-29 ]

Hi Sergei,

I have squashed all of my changes to fix this bug MDEV-10355 into a single commit b4a223e. Please review.

Thanks,
Jacob

Comment by Jacob Mathew (Inactive) [ 2017-04-19 ]

I have merged the final version of the fix, committed in https://github.com/MariaDB/server/commit/0b52b28b91eac1018d865f2f918b83c416565f2f, into the 10.2 branch.

Generated at Thu Feb 08 07:41:35 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.