[MDEV-19259] Separating Value from Item Created: 2019-04-16  Updated: 2023-05-04

Status: Stalled
Project: MariaDB Server
Component/s: Server
Fix Version/s: 11.2

Type: Task Priority: Major
Reporter: Oleksandr Byelkin Assignee: Oleksandr Byelkin
Resolution: Unresolved Votes: 0
Labels: None

Issue Links:
PartOf
is part of MDEV-6897 Making Items reentrant Open

 Description   

Make Value independent from Item, so Item can pass it and change (may be store also).

Why value

We're trying to solve the following goals by adding Value:

  1. A step to make Items reentrant
  2. Improve performance
  3. Reduce the code size by removing some data type specific functions/methods
  4. Pass data to plugins

The current value implementation

There is already struct st_value and a method in Item:

  bool save_in_value(THD *thd, struct st_value *value)
 
  {
 
    return type_handler()->Item_save_in_value(thd, this, value);
 
  }

Monty will write why the current implementation is bad for the intended goals.

For now it should be:

  • Null flag
  • reference on Type
  • DATA


 Comments   
Comment by Oleksandr Byelkin [ 2019-04-16 ]

first test is to make Item::val() method which return the value object and see how it will work in points where val_* used not in Item class.

Comment by Sergei Golubchik [ 2020-09-10 ]

I don't think "reference on Type" should be part of the Value. We already have problems with String and my_decimal that duplicate some of the Item's metadata.

Perhaps the Value object should have only the data and metadata should be in the Item?

Comment by Alexander Barkov [ 2020-09-11 ]

serg, this is a good idea. I have only one concern: how getting, say, a double value will look like.

{
 
  Value tmp;
 
  if (item->val(thd, &val))
 
  {
 
    handle_null();
 
  }
 
  else
 
  {
 
    double nr= item->type_handler()->value_to_double(tmp);
 
    handle_double(nr);
 
  }
 
}

Something like this?

It looks too cumbersome.

I suggest this API that I proposed by email:

{
 
  Double_null nr= item->to_double_null(thd);
 
  if (nr.is_null())
 
    handle_null();
 
  else
 
    handle_double(nr.val());
 
}

Note, Item::to_double_null() can use Value internally in the end:

Double_null Item::to_double_null()
 
{
 
  Value tmp;
 
  if (val(thd, &tmp))
 
    return Double_null(); // This is NULL
 
  return Double_null(type_handler()->value_to_double(val)); // A non-NULL value
 
}

The key point is that we can add methods like to_double_null() BEFORE adding Item::val(), thus split a huge change into smaller pieces:

1. Change val_real() to to_double_null()

  • a. Add to_double_null(), using the current API: val_real() and null_value,
    fix all callers to use to_double_null() instead of val_real()
  • b. Replace virtual val_real() to virtual to_double_null().

Note, 1a and 1b will be separate small patches.

Iterate these two steps for all non-reentrant methods (again, two patches for each method):
2. Change val_int() to to_longlong_null()
3. Change val_bool() to to_bool_null()
4. Change get_date() and get_time() to:

  • Date to_date()
  • Time to_time()
  • Datetime to_datetime()

5. Remove Item::null_value

After that, Item::val() can be added, if really needed, and all methods to_xxx_null(), to_date(), to_time(), to_datetime() can be modified to use val() internally.

Don't mix too many changes! One change at a time!

Comment by Alexander Barkov [ 2020-09-11 ]

How will addition look like with Item::val()?

bool real_op(Value *to)
 
{
 
  Value a, b;
 
  if (args[0]->val(&a) || args[1]->val(&b))
 
    return true;
 
  to->set_double(args[0]->type_handler()->value_to_double(a) +
 
                 args[1]->type_handler()->value_to_double(b));
 
}

The above is cumbersome again.

A much better version:

Double_null double_op()
 
{
 
  Double_null a= args[0]->to_double_null();
 
  Double_null b= args[1]->to_double_null();
 
  return Double_null(a.val() + b.val(), a.is_null() || b.is_null());
 
}

Even better version: why not add operator+() to Double_null and hide NULL processing inside, so the caller looks as simple as:

Double_null double_op()
 
{
 
  return a->to_double_null() +  b->to_double_null();
 
}

Comment by Michael Widenius [ 2020-09-11 ]

Just to get the discussion going:

Here is an example of how to separate the null value by adding a pointer to null object to all val functions, except val_str() and val_decimal() who don't need it (a null pointer is a good way to detect the null value for these)

diff --git a/sql/item_func.cc b/sql/item_func.cc
 
index ead72007518..2e10b211c53 100644
 
--- a/sql/item_func.cc
 
+++ b/sql/item_func.cc
 
@@ -1097,12 +1097,15 @@ void Item_real_typecast::print(String *str, enum_query_type query_type)
 
   str->append(')');
 
 }
 
 
-double Item_func_plus::real_op()
 
+double Item_func_plus::real_op(bool *null_value)
 
 {
 
-  double value= args[0]->val_real() + args[1]->val_real();
 
-  if ((null_value=args[0]->null_value || args[1]->null_value))
 
+  double value1= args[0]->val_real(null_value);
 
+  if (*null_value)
 
     return 0.0;
 
-  return check_float_overflow(value);
 
+  double value2= args[0]->val_real(null_value); 
+  if (*null_value)
 
+    return 0.0;
 
+  return check_float_overflow(value+value2);
 
 }
 
 
 
diff --git a/sql/item_func.h b/sql/item_func.h
 
index 3771992d617..dd1a9bfe8cd 100644
 
--- a/sql/item_func.h
 
+++ b/sql/item_func.h
 
@@ -739,13 +739,13 @@ class Item_func_hybrid_field_type: public Item_hybrid_func
 
   {
 
     return str_op_with_null_check(&str_value);
 
   }
 
-  longlong val_int_from_int_op()
 
+  longlong val_int_from_int_op(bool *null_value)
 
   {
 
-    return int_op();
 
+    return int_op(null_value);
 
   }
 
-  double val_real_from_real_op()
 
+  double val_real_from_real_op(bool *null_value)
 
   {
 
-    return real_op();
 
+    return real_op(null_value);
 
   }
 
 
   // Value methods that involve conversion
 
@@ -787,17 +787,17 @@ class Item_func_hybrid_field_type: public Item_hybrid_func
 
     Item_hybrid_func(thd, list)
 
   { collation= DTCollation_numeric(); }
 
 
-  double val_real()
 
+  double val_real(bool *null_value)
 
   {
 
     DBUG_ASSERT(fixed);
 
     return Item_func_hybrid_field_type::type_handler()->
 
-           Item_func_hybrid_field_type_val_real(this);
 
+      Item_func_hybrid_field_type_val_real(this, null_value);
 
   }
 
-  longlong val_int()
 
+  longlong val_int(bool *null_value)
 
   {
 
     DBUG_ASSERT(fixed);
 
     return Item_func_hybrid_field_type::type_handler()->
 
-           Item_func_hybrid_field_type_val_int(this);
 
+      Item_func_hybrid_field_type_val_int(this, null_value);
 
   }
 
   my_decimal *val_decimal(my_decimal *dec)
 
   {
 
@@ -1384,8 +1384,8 @@ class Item_func_plus :public Item_func_additive_op
 
   const char *func_name() const { return "+"; }
 
   enum precedence precedence() const { return ADD_PRECEDENCE; }
 
   bool fix_length_and_dec();
 
-  longlong int_op();
 
-  double real_op();
 
+  longlong int_op(bool *null_value);
 
+  double real_op(bool *null_value);
 
   my_decimal *decimal_op(my_decimal *);
 
   Item *get_copy(THD *thd)
 
   { return get_item_copy<Item_func_plus>(thd, this); }
 
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
 
index cbf620e85d1..09fc75a99db 100644
 
--- a/sql/sql_type.cc
 
+++ b/sql/sql_type.cc
 
@@ -5219,7 +5219,8 @@ Type_handler_int_result::Item_func_hybrid_field_type_val_str(
 
 
 double
 
 Type_handler_int_result::Item_func_hybrid_field_type_val_real(
 
-                                          Item_func_hybrid_field_type *item)
 
+                                         Item_func_hybrid_field_type *item,
 
+                                          bool *null_value)
 
                                           const
 
 {
 
   return item->val_real_from_int_op();
 
@@ -5227,11 +5228,11 @@ Type_handler_int_result::Item_func_hybrid_field_type_val_real(
 
 
 
 longlong
 
-Type_handler_int_result::Item_func_hybrid_field_type_val_int(
 
-                                          Item_func_hybrid_field_type *item)
 
+Type_handler_int_result::Item_func_hybrid_field_type_val_int(                                                             Item_func_hybrid_field_type *item,
 
+                                          bool *null_value)
 
                                           const
 
 {
 
-  return item->val_int_from_int_op();
 
+  return item->val_int_from_int_op(null_value);
 
 }

To usage of this would be:

bool null_value=0;   /* Only initialization needed in most cases */
 
longlong int= item->val_int(&null_value);

Note that the plus function is coded slightly different from before to allow shortcutting the + operation if we get a null early

Generated at Thu Feb 08 08:50:15 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.