Skip to content
  • Knut Anders Hatlen's avatar
    d5ab02a9
    Bug#21628161: CRASH/MEMORY CORRUPTION ADDING INDEXES TO VIRTUAL COLUMN · d5ab02a9
    Knut Anders Hatlen authored
    Bug#21688115: VIRTUAL COLUMN COMPUTATION SAVE_IN_FIELD() DID NOT
                  RETURN TRUE WITH DIVIDE 0
    
    When adding an index on a virtual generated column, the value of the
    virtual column needs to be computed so that it can be stored in the
    index. If an error or a warning was raised while computing the value,
    bad things could happen, such as crashes, assertions or insertion of
    corrupt data into the index.
    
    Bug#21671248 added a temporary workaround for these issues. This patch
    mainly cleans up that workaround, and adds tests to verify that the
    issues are fixed.
    
    The problem is caused by confusing return values from
    Item::save_in_field(), which is used by my_eval_gcolumn_expr_helper()
    to populate the data structures that hold the generated values. The
    code looked like this before the workaround in Bug#21671248:
    
          res= field->gcol_info->expr_item->save_in_field(field, 0);
          DBUG_ASSERT(!thd->is_error() || res);
    
          if (res)
            break;
    
    There are two issues:
    
    - In some cases save_in_field() returns non-zero (different from
      TYPE_OK) even though no error has happened. This could for example
      happen if the conversion of the generated value to the target type
      raises a warning, such as truncation or out-of-range. This makes the
      computation of the generated values stop, even though no error has
      actually occurred, which causes problems further down the road
      (Bug#21628161). Symptoms before Bug#21671248 were crashes or wrong
      results because the index creation went on using the not fully
      computed values.
    
    - In some cases save_in_field() returns TYPE_OK (zero), but
      thd->is_error() returns true and indicates that something has gone
      wrong. This causes an assertion failure (Bug#21688115). (The
      assertion was disabled temporarily by Bug#21671248.)
    
    The first issue, where Item::save_in_field() returns non-zero even if
    no error has happened, seems to be per design. The function returns a
    value of type type_conversion_status, which is an enum that has
    non-zero status codes for warnings and notes as well as errors. The
    patch fixes this by additionally checking thd->is_error() if
    save_in_field() returns non-zero. If thd->is_error() returns false, we
    continue with the computation of the virtual column.
    
    The second issue, where Item::save_in_field() returns TYPE_OK even if
    an error has happened, could also be fixed by checking thd->is_error()
    in my_eval_gcolumn_expr_helper(). However, save_in_field() already
    checks is_error() before it returns and adjusts the return value
    accordingly. It's just that it doesn't check it on every return point.
    The patch fixes this by reorganizing Item::save_in_field() so that it
    checks is_error() regardless of how it returns. It does this by
    renaming Item::save_in_field() and all its overrides to
    save_in_field_inner(), and adding a new non-virtual
    Item::save_in_field() function that calls save_in_field_inner() and
    then checks THD::is_error() before returning. This ensures that the
    error handling in save_in_field() is the same in all subclasses of
    Item.
    d5ab02a9
    Bug#21628161: CRASH/MEMORY CORRUPTION ADDING INDEXES TO VIRTUAL COLUMN
    Knut Anders Hatlen authored
    Bug#21688115: VIRTUAL COLUMN COMPUTATION SAVE_IN_FIELD() DID NOT
                  RETURN TRUE WITH DIVIDE 0
    
    When adding an index on a virtual generated column, the value of the
    virtual column needs to be computed so that it can be stored in the
    index. If an error or a warning was raised while computing the value,
    bad things could happen, such as crashes, assertions or insertion of
    corrupt data into the index.
    
    Bug#21671248 added a temporary workaround for these issues. This patch
    mainly cleans up that workaround, and adds tests to verify that the
    issues are fixed.
    
    The problem is caused by confusing return values from
    Item::save_in_field(), which is used by my_eval_gcolumn_expr_helper()
    to populate the data structures that hold the generated values. The
    code looked like this before the workaround in Bug#21671248:
    
          res= field->gcol_info->expr_item->save_in_field(field, 0);
          DBUG_ASSERT(!thd->is_error() || res);
    
          if (res)
            break;
    
    There are two issues:
    
    - In some cases save_in_field() returns non-zero (different from
      TYPE_OK) even though no error has happened. This could for example
      happen if the conversion of the generated value to the target type
      raises a warning, such as truncation or out-of-range. This makes the
      computation of the generated values stop, even though no error has
      actually occurred, which causes problems further down the road
      (Bug#21628161). Symptoms before Bug#21671248 were crashes or wrong
      results because the index creation went on using the not fully
      computed values.
    
    - In some cases save_in_field() returns TYPE_OK (zero), but
      thd->is_error() returns true and indicates that something has gone
      wrong. This causes an assertion failure (Bug#21688115). (The
      assertion was disabled temporarily by Bug#21671248.)
    
    The first issue, where Item::save_in_field() returns non-zero even if
    no error has happened, seems to be per design. The function returns a
    value of type type_conversion_status, which is an enum that has
    non-zero status codes for warnings and notes as well as errors. The
    patch fixes this by additionally checking thd->is_error() if
    save_in_field() returns non-zero. If thd->is_error() returns false, we
    continue with the computation of the virtual column.
    
    The second issue, where Item::save_in_field() returns TYPE_OK even if
    an error has happened, could also be fixed by checking thd->is_error()
    in my_eval_gcolumn_expr_helper(). However, save_in_field() already
    checks is_error() before it returns and adjusts the return value
    accordingly. It's just that it doesn't check it on every return point.
    The patch fixes this by reorganizing Item::save_in_field() so that it
    checks is_error() regardless of how it returns. It does this by
    renaming Item::save_in_field() and all its overrides to
    save_in_field_inner(), and adding a new non-virtual
    Item::save_in_field() function that calls save_in_field_inner() and
    then checks THD::is_error() before returning. This ensures that the
    error handling in save_in_field() is the same in all subclasses of
    Item.
Loading