Comment 3 for bug 337038

Revision history for this message
Padraig O'Sullivan (posulliv) wrote :

Ok, I take my above comment back - I was looking at the wrong piece of code. I've traced it to the relevant part now though. Basically, the following is executed in the file drizzled/field/decimal.cc in the Field_new_decimal::store_value() function:

  if (warn_if_overflow(my_decimal2binary(E_DEC_FATAL_ERROR & ~E_DEC_OVERFLOW,
                                         decimal_value, ptr, precision, dec)))
  {
    my_decimal buff;
    set_value_on_overflow(&buff, decimal_value->sign());
    my_decimal2binary(E_DEC_FATAL_ERROR, &buff, ptr, precision, dec);
    error= 1;
  }

where warn_if_overflow() looks like:

/**
  Process decimal library return codes and issue warnings for overflow and
  truncation.

  @param op_result decimal library return code (E_DEC_* see include/decimal.h)

  @retval
    1 there was overflow
  @retval
    0 no error or some other errors except overflow
*/

int Field::warn_if_overflow(int op_result)
{
  if (op_result == E_DEC_OVERFLOW)
  {
    set_warning(DRIZZLE_ERROR::WARN_LEVEL_WARN, ER_WARN_DATA_OUT_OF_RANGE, 1);
    return 1;
  }
  if (op_result == E_DEC_TRUNCATED)
  {
    set_warning(DRIZZLE_ERROR::WARN_LEVEL_NOTE, ER_WARN_DATA_TRUNCATED, 1);
    /* We return 0 here as this is not a critical issue */
  }
  return 0;
}

So you can see that an error is being returned for overflow but not for truncation. The fix I'm suggesting at the moment is to remove the call to warn_if_overflow() since I think its useless here as Drizzle should always error on data issues. Basically, my fix is to do the following in Field_new_decimal::store_value():

  if ((error = my_decimal2binary(E_DEC_FATAL_ERROR & ~E_DEC_OVERFLOW,
                                 decimal_value, ptr, precision, dec)) != E_DEC_OK)
  {
    my_decimal buff;
    set_value_on_overflow(&buff, decimal_value->sign());
    my_decimal2binary(E_DEC_FATAL_ERROR, &buff, ptr, precision, dec);
    error= 1;
  }

Now, when running the above create table statement we get the following behavior:

drizzle> use test
Database changed
drizzle> create table t1 (
    -> f1 decimal not null default 17.49,
    -> f2 decimal not null default 17.68,
    -> f3 decimal not null default 99.2,
    -> f4 decimal not null default 99.7,
    -> f5 decimal not null default 104.49,
    -> f6 decimal not null default 199.91,
    -> f7 decimal not null default 999.9,
    -> f8 decimal not null default 9999.99);
ERROR 1067 (42000): Invalid default value for 'f1'
drizzle>

Is this the kind of behavior we are looking for here? I wasn't sure if this is the right behavior since the error message doesn't mention that the value is invalid because it is truncated. Any thoughts?

-Padraig