Skip to content

Commit

Permalink
fix(tianmu): crashed when insert out of range #1716
Browse files Browse the repository at this point in the history
[summary]
In non-strict sql_mode, insert sucess with warning.
in strict sql_mode, crashed in function `Sql_cmd_insert_values::execute_inner(THD *thd)` in file `sql/sql_insert.cc`:
```
      if (write_record(thd, insert_table, &info, &update)) {
        has_error = true;
        break;
      }
      thd->get_stmt_da()->inc_current_row_for_condition();
    }
  }  // Statement plan is available within these braces

  assert(has_error == thd->get_stmt_da()->is_error());
```

here `has_error` = false, `thd->get_stmt_da()->is_error()` = true, that
caused crashed.

When insert data out of range, `tianmu` engine push a warning into mysql,  but in strict sql_mode, the
`Sql_condition::SL_WARNING` will be changed to `Sql_condition::SL_ERROR` in function:

```
/**
  Implementation of STRICT mode.
  Upgrades a set of given conditions from warning to error.
*/
bool Strict_error_handler::handle_condition(
...
...
switch (sql_errno) {
    ...
    case ER_WARN_DATA_OUT_OF_RANGE:
    case ER_WARN_DATA_OUT_OF_RANGE_FUNCTIONAL_INDEX:
      if ((*level == Sql_condition::SL_WARNING) &&
          (!thd->get_transaction()->cannot_safely_rollback(
               Transaction_ctx::STMT) ||
           (thd->variables.sql_mode & MODE_STRICT_ALL_TABLES))) {
        (*level) = Sql_condition::SL_ERROR;
      }
      break;
    default:
      break;
  }
  return false;
```

So, `write_record(thd, insert_table, &info, &update)` should return error and then set ` has_error = true;`, after debug, I found `ret` value not set `1` when execption get in function:

```
int Engine::InsertRow(const std::string &table_path, [[maybe_unused]] Transaction *trans_, TABLE *table,
                      std::shared_ptr<TableShare> &share) {
  int ret = 0;
  try {
    ret = rct->Insert(table);
    return ret;
  } catch (...) {
    TIANMU_LOG(LogCtl_Level::ERROR, "delayed inserting failed.");
  }

  return ret;
}
```

we should set `ret` = 1 in strict sql_mode in catch block.
  • Loading branch information
hustjieke authored and mergify[bot] committed May 24, 2023
1 parent a374a07 commit d5c1051
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 4 deletions.
1 change: 0 additions & 1 deletion mysql-test/suite/tianmu/r/out_of_range_issue1151.result
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
drop database if exists out_of_range_issue1151;
create database test;
create database out_of_range_issue1151;
use out_of_range_issue1151;
create table tiny(a tinyint, b tinyint unsigned) engine = tianmu DEFAULT CHARSET=utf8mb4;
Expand Down
16 changes: 14 additions & 2 deletions storage/tianmu/common/common_definitions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

#include "common_definitions.h"
#include "exception.h"

#include <iomanip>
#include <mutex>
Expand All @@ -40,47 +41,58 @@ void PushWarningIfOutOfRange(THD *thd, std::string col_name, int64_t v, int type
if (unsigned_flag && (static_cast<uint64_t>(v) > TIANMU_TINYINT_MAX)) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, 0, TIANMU_TINYINT_MAX, unsigned_flag, v).c_str());
throw std::exception();
throw common::OutOfRangeException(getErrMsg(col_name, 0, TIANMU_TINYINT_MAX, unsigned_flag, v));
} else if (v > TIANMU_TINYINT_MAX || v < TIANMU_TINYINT_MIN) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, TIANMU_TINYINT_MIN, TIANMU_TINYINT_MAX, unsigned_flag, v).c_str());
throw std::exception();
throw common::OutOfRangeException(
getErrMsg(col_name, TIANMU_TINYINT_MIN, TIANMU_TINYINT_MAX, unsigned_flag, v));
}
} break;
case 2: { // MYSQL_TYPE_SHORT
if (unsigned_flag && (static_cast<uint64_t>(v) > TIANMU_SMALLINT_MAX)) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, 0, TIANMU_SMALLINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, 0, TIANMU_SMALLINT_MAX, unsigned_flag, v));
} else if (v > TIANMU_SMALLINT_MAX || v < TIANMU_SMALLINT_MIN) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, TIANMU_SMALLINT_MIN, TIANMU_SMALLINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(
getErrMsg(col_name, TIANMU_SMALLINT_MIN, TIANMU_SMALLINT_MAX, unsigned_flag, v));
};
} break;
case 9: { // MYSQL_TYPE_INT24
if (unsigned_flag && (static_cast<uint64_t>(v) > TIANMU_MEDIUMINT_MAX)) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, 0, TIANMU_MEDIUMINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, 0, TIANMU_MEDIUMINT_MAX, unsigned_flag, v));
} else if (v > TIANMU_MEDIUMINT_MAX || v < TIANMU_MEDIUMINT_MIN) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, TIANMU_MEDIUMINT_MIN, TIANMU_MEDIUMINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(
getErrMsg(col_name, TIANMU_MEDIUMINT_MIN, TIANMU_MEDIUMINT_MAX, unsigned_flag, v));
}
} break;
case 3: { // MYSQL_TYPE_LONG
if (unsigned_flag && (static_cast<uint64_t>(v) > TIANMU_INT_MAX)) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, 0, TIANMU_INT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, 0, TIANMU_INT_MAX, unsigned_flag, v));
} else if (v > TIANMU_INT_MAX || v < TIANMU_INT_MIN) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, TIANMU_INT_MIN, TIANMU_INT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, TIANMU_INT_MIN, TIANMU_INT_MAX, unsigned_flag, v));
}
} break;
case 8: { // MYSQL_TYPE_LONGLONG
if (unsigned_flag && (static_cast<uint64_t>(v) > TIANMU_BIGINT_MAX)) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, 0, TIANMU_BIGINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, 0, TIANMU_BIGINT_MAX, unsigned_flag, v));
} else if (v > TIANMU_BIGINT_MAX || v < TIANMU_BIGINT_MIN) {
PushWarning(thd, Sql_condition::SL_WARNING, ER_WARN_DATA_OUT_OF_RANGE,
getErrMsg(col_name, TIANMU_BIGINT_MIN, TIANMU_BIGINT_MAX, unsigned_flag, v).c_str());
throw common::OutOfRangeException(getErrMsg(col_name, TIANMU_BIGINT_MIN, TIANMU_BIGINT_MAX, unsigned_flag, v));
}
} break;
default: // For type which is not integer, nothing to do
Expand Down
5 changes: 5 additions & 0 deletions storage/tianmu/common/exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ class AutoIncException : public Exception {
AutoIncException(std::string const &msg) : Exception(msg) {}
};

class OutOfRangeException : public Exception {
public:
OutOfRangeException(std::string const &msg) : Exception(msg) {}
};

} // namespace common
} // namespace Tianmu

Expand Down
9 changes: 8 additions & 1 deletion storage/tianmu/core/engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1478,11 +1478,18 @@ int Engine::InsertRow(const std::string &table_path, [[maybe_unused]] Transactio
ret = rct->Insert(table);
}
return ret;
} catch (common::OutOfRangeException &e) {
TIANMU_LOG(LogCtl_Level::ERROR, "inserting data out of range. %s %s", e.what(), e.trace().c_str());
// in strict sql_mode, we should return error no.
// TODO: in the future, we'll support insert success with warning in strict sql_mode, currently the data is
// not write into data file, refs crashed issue: https://github.com/stoneatom/stonedb/issues/1716
if (trans_->Thd()->is_strict_mode()) {
ret = 1;
}
} catch (common::Exception &e) {
TIANMU_LOG(LogCtl_Level::ERROR, "delayed inserting failed. %s %s", e.what(), e.trace().c_str());
} catch (std::exception &e) {
TIANMU_LOG(LogCtl_Level::ERROR, "delayed inserting failed. %s", e.what());
ret = 1;
} catch (...) {
TIANMU_LOG(LogCtl_Level::ERROR, "delayed inserting failed.");
}
Expand Down

0 comments on commit d5c1051

Please sign in to comment.