Skip to content

Commit

Permalink
MyRocks: cleanup Rdb_convert_to_record_key_decoder class
Browse files Browse the repository at this point in the history 
- Since this class is a non-instantiable collection of static methods, delete
  the rest of special methods (default constructor, move constructor, move
  assignment operator, destructor) in addition to the currently deleted ones.
- Convert method arguments from pointers to references and propagate this up and
  down the call hierarchy. Most notably this converts many "TABLE *" uses to
  "const TABLE &" throughout MyRocks, deletes a bit of dead code, converts some
  constructor assignments to initializer list items.
- For touched methods, add [[nodiscard]] as applicable and remove redundant
  const from parameters, do other minor cleanups.
- Move struct Rdb_unpack_func_context definition from rdb_datadic.h to
  rdb_datadic.cc leaving a forward declaration in the header file.
  • Loading branch information
@laurynas-biveinis
laurynas-biveinis committed Oct 24, 2024
1 parent b72c7e5 commit 6819ca4
Show file tree
Hide file tree
Showing 11 changed files with 538 additions and 541 deletions.
94 changes: 48 additions & 46 deletions storage/rocksdb/ha_rocksdb.cc
View file

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions storage/rocksdb/ha_rocksdb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -884,10 +884,9 @@ class ha_rocksdb : public my_core::handler, public blob_buffer {
bool contains_foreign_key()
MY_ATTRIBUTE((__nonnull__, __warn_unused_result__));

int inplace_populate_sk(
TABLE *const table_arg,
const std::unordered_set<std::shared_ptr<Rdb_key_def>> &indexes)
MY_ATTRIBUTE((__nonnull__, __warn_unused_result__));
[[nodiscard]] int inplace_populate_sk(
const TABLE &table_arg,
const std::unordered_set<std::shared_ptr<Rdb_key_def>> &indexes);

void inc_table_n_rows();
void dec_table_n_rows();
Expand Down Expand Up @@ -972,10 +971,11 @@ class ha_rocksdb : public my_core::handler, public blob_buffer {
int create(const char *const name, TABLE *const form,
HA_CREATE_INFO *const create_info, dd::Table *table_def) override
MY_ATTRIBUTE((__warn_unused_result__));
int truncate_table(Rdb_tbl_def *tbl_def,
const std::string &actual_user_table_name,
TABLE *table_arg, ulonglong auto_increment_value,
dd::Table *table_def);
[[nodiscard]] int truncate_table(Rdb_tbl_def *tbl_def,
const std::string &actual_user_table_name,
const TABLE &table_arg,
ulonglong auto_increment_value,
dd::Table *table_def);
int delete_all_rows() override;
bool check_if_incompatible_data(HA_CREATE_INFO *const info,
uint table_changes) override
Expand Down
28 changes: 14 additions & 14 deletions storage/rocksdb/nosql_access.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1461,17 +1461,16 @@ class select_exec {
public:
explicit select_exec(const base_select_parser &parser,
const base_protocol &protocol)
: m_parser(parser), m_protocol(protocol) {
m_table = parser.get_table();
m_table_share = m_table->s;
: m_parser(parser), m_protocol(protocol), m_table(*parser.get_table()) {
m_table_share = m_table.s;
m_index = parser.get_index();
m_index_info = &m_table->key_info[m_index];
m_pk_info = &m_table->key_info[m_table_share->primary_key];
m_index_info = &m_table.key_info[m_index];
m_pk_info = &m_table.key_info[m_table_share->primary_key];
m_is_point_query = true;
m_start_full_key = m_end_full_key = true;
m_ddl_manager = rdb_get_ddl_manager();
m_index_is_pk = (m_index == m_table_share->primary_key);
m_handler = static_cast<ha_rocksdb *>(m_table->file);
m_handler = static_cast<ha_rocksdb *>(m_table.file);
m_thd = parser.get_thd();
m_examined_rows = 0;
m_rows_sent = 0;
Expand Down Expand Up @@ -1611,15 +1610,15 @@ class select_exec {

// Since we bypassed prepare function, read_set might not be set.
// Let's set it before calling eval_cond().
bitmap_set_bit(m_table->read_set, field->field_index());
bitmap_set_bit(m_table.read_set, field->field_index());
}
return false;
}

private:
const base_select_parser &m_parser;
const base_protocol &m_protocol;
TABLE *m_table;
const TABLE &m_table;
TABLE_SHARE *m_table_share;
Rdb_ddl_manager *m_ddl_manager;
Rdb_tbl_def *m_tbl_def;
Expand Down Expand Up @@ -2197,7 +2196,7 @@ void INLINE_ATTR select_exec::scan_value() {
const auto &field_list = m_parser.get_field_list();
for (uint i = 0; i < field_list.size(); i++) {
// bitmap is not set yet because we skips prepare function
bitmap_set_bit(m_table->read_set, field_list[i]->field_index());
bitmap_set_bit(m_table.read_set, field_list[i]->field_index());
}

std::vector<bool> index_cover_bitmap(m_table_share->fields, false);
Expand All @@ -2209,7 +2208,7 @@ void INLINE_ATTR select_exec::scan_value() {

m_keyread_only = true;
for (uint i = 0; i < m_table_share->fields; i++) {
if (bitmap_is_set(m_table->read_set, i) && !index_cover_bitmap[i]) {
if (bitmap_is_set(m_table.read_set, i) && !index_cover_bitmap[i]) {
m_keyread_only = false;
break;
}
Expand All @@ -2219,7 +2218,7 @@ void INLINE_ATTR select_exec::scan_value() {
m_key_def->get_lookup_bitmap(m_table, &m_lookup_bitmap);
}

m_converter->setup_field_decoders(m_table->read_set, m_index,
m_converter->setup_field_decoders(m_table.read_set, m_index,
false /* keyread_only */);
}

Expand Down Expand Up @@ -2492,7 +2491,8 @@ bool INLINE_ATTR select_exec::eval_cond() {
bool INLINE_ATTR select_exec::unpack_for_pk(const rocksdb::Slice &rkey,
const rocksdb::Slice &rvalue) {
// decode will handle key/value decoding for PK
int rc = m_converter->decode(*m_key_def, m_table->record[0], &rkey, &rvalue);
const auto rc =
m_converter->decode(*m_key_def, m_table.record[0], &rkey, &rvalue);
if (rc) {
m_handler->print_error(rc, 0);
return true;
Expand All @@ -2514,7 +2514,7 @@ bool INLINE_ATTR select_exec::unpack_for_sk(txn_wrapper *txn,
if (covers_lookup) {
// SK covers the entire lookup
rc =
m_key_def->unpack_record(m_table, m_table->record[0], &rkey, &rvalue,
m_key_def->unpack_record(m_table, m_table.record[0], &rkey, &rvalue,
m_converter->get_verify_row_debug_checksums());
if (rc) {
m_handler->print_error(rc, 0);
Expand Down Expand Up @@ -2544,7 +2544,7 @@ bool INLINE_ATTR select_exec::unpack_for_sk(txn_wrapper *txn,
return true;
}

rc = m_converter->decode(*m_pk_def, m_table->record[0], &pk_key, &m_pk_value);
rc = m_converter->decode(*m_pk_def, m_table.record[0], &pk_key, &m_pk_value);
if (rc) {
m_handler->print_error(rc, 0);
return true;
Expand Down
81 changes: 39 additions & 42 deletions storage/rocksdb/rdb_converter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ int Rdb_convert_to_record_value_decoder::decode_instant(
0 OK
other HA_ERR error code (can be SE-specific)
*/
int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, TABLE *table,
int Rdb_convert_to_record_value_decoder::decode(uchar *buf, const TABLE &table,
Rdb_field_encoder *field_dec,
Rdb_string_reader *reader,
bool decode, bool is_null) {
Expand All @@ -122,7 +122,7 @@ int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, TABLE *table,
Besides that, set the field value to default value. CHECKSUM TABLE
depends on this.
*/
memcpy(ptr, table->s->default_values + field_dec->m_field_offset,
memcpy(ptr, table.s->default_values + field_dec->m_field_offset,
field_dec->m_field_pack_length);
}
} else {
Expand All @@ -134,7 +134,7 @@ int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, TABLE *table,
if (!field_dec->m_is_virtual_gcol) {
if (field_dec->m_field_type == MYSQL_TYPE_BLOB ||
field_dec->m_field_type == MYSQL_TYPE_JSON) {
err = decode_blob(table, ptr, field_dec, reader, decode);
err = decode_blob(ptr, field_dec, reader, decode);
} else if (field_dec->m_field_type == MYSQL_TYPE_VARCHAR) {
err = decode_varchar(ptr, field_dec, reader, decode);
} else {
Expand All @@ -157,8 +157,8 @@ int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, TABLE *table,
other HA_ERR error code (can be SE-specific)
*/
int Rdb_convert_to_record_value_decoder::decode_blob(
TABLE *table MY_ATTRIBUTE((__unused__)), uchar *const buf,
Rdb_field_encoder *field_dec, Rdb_string_reader *reader, bool decode) {
uchar *buf, Rdb_field_encoder *field_dec, Rdb_string_reader *reader,
bool decode) {
// Get the number of bytes needed to store length
const uint length_bytes =
field_dec->m_field_pack_length - portable_sizeof_char_ptr;
Expand Down Expand Up @@ -262,14 +262,12 @@ int Rdb_convert_to_record_value_decoder::decode_varchar(

template <typename value_field_decoder, typename dst_type>
Rdb_value_field_iterator<value_field_decoder, dst_type>::
Rdb_value_field_iterator(TABLE *table,
Rdb_value_field_iterator(const TABLE &table,
Rdb_string_reader *value_slice_reader,
const Rdb_converter *rdb_converter, dst_type buf)
: m_buf(buf) {
assert(table != nullptr);
: m_table(table), m_buf(buf) {
assert(buf != nullptr);

m_table = table;
m_value_slice_reader = value_slice_reader;
auto fields = rdb_converter->get_decode_fields();
m_field_iter = fields->begin();
Expand Down Expand Up @@ -360,11 +358,10 @@ bool Rdb_value_field_iterator<value_field_decoder, dst_type>::is_null() const {
@param table IN Current open table
*/
Rdb_converter::Rdb_converter(const THD *thd, const Rdb_tbl_def *tbl_def,
TABLE *table, const dd::Table *dd_table)
const TABLE &table, const dd::Table *dd_table)
: m_thd(thd), m_tbl_def(tbl_def), m_table(table) {
assert(thd != nullptr);
assert(tbl_def != nullptr);
assert(table != nullptr);
m_key_requested = false;
m_verify_row_debug_checksums = false;
m_maybe_unpack_info = false;
Expand All @@ -374,7 +371,7 @@ Rdb_converter::Rdb_converter(const THD *thd, const Rdb_tbl_def *tbl_def,
}

Rdb_converter::~Rdb_converter() {
for (uint i = 0; i < m_table->s->fields; i++) {
for (uint i = 0; i < m_table.s->fields; i++) {
my_free(m_encoder_arr[i].m_instant_default_value);
}
my_free(m_encoder_arr);
Expand All @@ -390,7 +387,7 @@ Rdb_converter::~Rdb_converter() {
void Rdb_converter::get_storage_type(Rdb_field_encoder *const encoder,
const uint kp) {
auto pk_descr =
m_tbl_def->m_key_descr_arr[ha_rocksdb::pk_index(*m_table, *m_tbl_def)];
m_tbl_def->m_key_descr_arr[ha_rocksdb::pk_index(m_table, *m_tbl_def)];

// STORE_SOME uses unpack_info.
if (pk_descr->has_unpack_info(kp)) {
Expand Down Expand Up @@ -431,26 +428,26 @@ void Rdb_converter::setup_field_decoders(const MY_BITMAP *field_map,
// We also have to load the fields that are required to decode the requested
// virtual fields.

std::vector<bool> bases(m_table->s->fields);
std::vector<bool> bases(m_table.s->fields);

for (uint i = 0; i < m_table->s->fields; i++) {
for (uint i = 0; i < m_table.s->fields; i++) {
const bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index());
bitmap_is_set(field_map, m_table.field[i]->field_index());

if (field_requested && m_table->field[i]->is_virtual_gcol()) {
for (uint j = 0; j < m_table->s->fields; j++) {
if (bitmap_is_set(&m_table->field[i]->gcol_info->base_columns_map, j)) {
if (field_requested && m_table.field[i]->is_virtual_gcol()) {
for (uint j = 0; j < m_table.s->fields; j++) {
if (bitmap_is_set(&m_table.field[i]->gcol_info->base_columns_map, j)) {
bases[j] = true;
}
}
}
}

for (uint i = 0; i < m_table->s->fields; i++) {
for (uint i = 0; i < m_table.s->fields; i++) {
bool field_requested =
decode_all_fields || m_verify_row_debug_checksums ||
bitmap_is_set(field_map, m_table->field[i]->field_index()) || bases[i];
bitmap_is_set(field_map, m_table.field[i]->field_index()) || bases[i];

// We only need the decoder if the whole record is stored.
if (m_encoder_arr[i].m_storage_type != Rdb_field_encoder::STORE_ALL) {
Expand Down Expand Up @@ -486,7 +483,7 @@ void Rdb_converter::setup_field_decoders(const MY_BITMAP *field_map,
m_decoders_vect.erase(m_decoders_vect.begin() + last_useful,
m_decoders_vect.end());

if (!keyread_only && active_index != m_table->s->primary_key) {
if (!keyread_only && active_index != m_table.s->primary_key) {
m_tbl_def->m_key_descr_arr[active_index]->get_lookup_bitmap(
m_table, &m_lookup_bitmap);
}
Expand All @@ -498,7 +495,7 @@ void Rdb_converter::setup_field_encoders(const dd::Table *dd_table) {

m_encoder_arr = static_cast<Rdb_field_encoder *>(
my_malloc(PSI_NOT_INSTRUMENTED,
m_table->s->fields * sizeof(Rdb_field_encoder), MYF(0)));
m_table.s->fields * sizeof(Rdb_field_encoder), MYF(0)));
if (m_encoder_arr == nullptr) {
return;
}
Expand All @@ -516,8 +513,8 @@ void Rdb_converter::setup_field_encoders(const dd::Table *dd_table) {
}
}

for (uint i = 0; i < m_table->s->fields; i++) {
Field *const field = m_table->field[i];
for (uint i = 0; i < m_table.s->fields; i++) {
const auto *const field = m_table.field[i];
m_encoder_arr[i].m_storage_type = Rdb_field_encoder::STORE_ALL;

/*
Expand All @@ -530,8 +527,8 @@ void Rdb_converter::setup_field_encoders(const dd::Table *dd_table) {
If hidden pk exists, we skip this check since the field will never be
part of the hidden pk.
*/
if (!Rdb_key_def::table_has_hidden_pk(*m_table)) {
KEY *const pk_info = &m_table->key_info[m_table->s->primary_key];
if (!Rdb_key_def::table_has_hidden_pk(m_table)) {
const auto *const pk_info = &m_table.key_info[m_table.s->primary_key];
for (uint kp = 0; kp < pk_info->user_defined_key_parts; kp++) {
// key_part->fieldnr is counted from 1
if (field->field_index() + 1 == pk_info->key_part[kp].fieldnr) {
Expand All @@ -557,7 +554,7 @@ void Rdb_converter::setup_field_encoders(const dd::Table *dd_table) {
m_encoder_arr[i].m_field_type = field_type;
m_encoder_arr[i].m_field_index = i;
m_encoder_arr[i].m_field_pack_length = field->pack_length();
m_encoder_arr[i].m_field_offset = field->field_ptr() - m_table->record[0];
m_encoder_arr[i].m_field_offset = field->field_ptr() - m_table.record[0];
m_encoder_arr[i].m_is_virtual_gcol = field->is_virtual_gcol();

if (field_type == MYSQL_TYPE_VARCHAR) {
Expand Down Expand Up @@ -610,8 +607,8 @@ void Rdb_converter::setup_field_encoders(const dd::Table *dd_table) {
// update ha_rocksdb::rocksdb_support_instant()
// For temporary table, its null_fields value maybe different than normal
// table. see create_tmp_table()
assert(m_table->s->table_category == TABLE_CATEGORY_TEMPORARY ||
ceil((double)m_table->s->null_fields / 8) ==
assert(m_table.s->table_category == TABLE_CATEGORY_TEMPORARY ||
ceil((double)m_table.s->null_fields / 8) ==
(uint)m_null_bytes_length_in_record);
}

Expand Down Expand Up @@ -848,7 +845,7 @@ int Rdb_converter::encode_value_slice(
char *const data = const_cast<char *>(m_storage_record.ptr());
if (has_ttl_column) {
assert(pk_def.get_ttl_field_index() != UINT_MAX);
const auto *const field = m_table->field[pk_def.get_ttl_field_index()];
const auto *const field = m_table.field[pk_def.get_ttl_field_index()];
assert(field->real_type() == MYSQL_TYPE_LONGLONG ||
field->type() == MYSQL_TYPE_TIMESTAMP);

Expand Down Expand Up @@ -915,14 +912,14 @@ int Rdb_converter::encode_value_slice(
m_storage_record.append(reinterpret_cast<char *>(pk_unpack_info->ptr()),
pk_unpack_info->get_current_pos());
}
for (uint i = 0; i < m_table->s->fields; i++) {
for (uint i = 0; i < m_table.s->fields; i++) {
Rdb_field_encoder &encoder = m_encoder_arr[i];
/* Don't pack decodable PK key parts */
if (encoder.m_storage_type != Rdb_field_encoder::STORE_ALL) {
continue;
}

Field *const field = m_table->field[i];
const auto *const field = m_table.field[i];

if (field->is_virtual_gcol()) {
continue;
Expand All @@ -943,22 +940,21 @@ int Rdb_converter::encode_value_slice(

if (encoder.m_field_type == MYSQL_TYPE_BLOB ||
encoder.m_field_type == MYSQL_TYPE_JSON) {
my_core::Field_blob *blob =
reinterpret_cast<my_core::Field_blob *>(field);
const auto *const blob = reinterpret_cast<const Field_blob *>(field);
/* Get the number of bytes needed to store length*/
const uint length_bytes = blob->pack_length() - portable_sizeof_char_ptr;

/* Store the length of the value */
m_storage_record.append(reinterpret_cast<char *>(blob->field_ptr()),
m_storage_record.append(reinterpret_cast<const char *>(blob->field_ptr()),
length_bytes);

/* Store the blob value itself */
char *data_ptr;
memcpy(&data_ptr, blob->field_ptr() + length_bytes, sizeof(uchar **));
m_storage_record.append(data_ptr, blob->get_length());
} else if (encoder.m_field_type == MYSQL_TYPE_VARCHAR) {
Field_varstring *const field_var =
reinterpret_cast<Field_varstring *>(field);
const auto *const field_var =
reinterpret_cast<const Field_varstring *>(field);
uint data_len;
/* field_var->get_length_bytes() is 1 or 2 */
if (field_var->get_length_bytes() == 1) {
Expand All @@ -967,13 +963,14 @@ int Rdb_converter::encode_value_slice(
assert(field_var->get_length_bytes() == 2);
data_len = uint2korr(field_var->field_ptr());
}
m_storage_record.append(reinterpret_cast<char *>(field_var->field_ptr()),
field_var->get_length_bytes() + data_len);
m_storage_record.append(
reinterpret_cast<const char *>(field_var->field_ptr()),
field_var->get_length_bytes() + data_len);
} else {
/* Copy the field data */
const uint len = field->pack_length();
m_storage_record.append(reinterpret_cast<char *>(field->field_ptr()),
len);
m_storage_record.append(
reinterpret_cast<const char *>(field->field_ptr()), len);
}
}

Expand Down
Loading

0 comments on commit 6819ca4

Please sign in to comment.