Skip to content

Commit

Permalink
Fix MyRocks compilation issues
Browse files Browse the repository at this point in the history
Summary:
1. Suppress warnings only if compiler supports "-Wno-xxxxxx".
2. Fix issues described at https://jira.percona.com/browse/PS-6054 and https://jira.percona.com/browse/PS-6058
3. Fix linking issues for clang-7 and clang-8:
```
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 9 refers to earlier section 3
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 10 refers to earlier section 4
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 11 refers to earlier section 5
```
4. Fix `-Werror=ignored-qualifiers` warnings e.g.
```
/fb-8.0.17/storage/rocksdb/ha_rocksdb.cc:882:70: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
  882 |       static_cast<const rocksdb::InfoLogLevel>(rocksdb_info_log_level));
```
5. Fix a `-Wundefined-reinterpret-cast` warning:
```
/fb-8.0.17/storage/rocksdb/ha_rocksdb.cc:3777:11: error: dereference of type 'myrocks::Rdb_transaction **' that was reinterpret_cast from type 'void **' has undefined behavior [-Werror,-Wundefined-reinterpret-cast]
  return *reinterpret_cast<Rdb_transaction **>(
```
6. Fix a `-Werror=attributes` warning:
```
/fb-8.0.17/include/my_compiler.h:100:40: error: ‘nonnull’ attribute only applies to function types [-Werror=attributes]
  100 | #define MY_ATTRIBUTE(A) __attribute__(A)
```
7. Fix
```
/fb-8.0.17/sql/dd/impl/types/schema_impl.cc:100:1: error: base class ‘class dd::Weak_object’ should be explicitly initialized in the copy constructor [-Werror=extra]
 Schema_impl::Schema_impl(const Schema_impl &src)
```
Pull Request resolved: facebook#1076

Reviewed By: lloyd

Differential Revision: D19301431

Pulled By: lth
  • Loading branch information
inikep committed Mar 28, 2023
1 parent 80f7679 commit e132d68
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 42 deletions.
26 changes: 13 additions & 13 deletions storage/rocksdb/ha_rocksdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "sql/sql_class.h"
#include "sql/sql_lex.h"
#include "sql/sql_table.h"
#include "sql/sql_thd_internal_api.h"

/* RocksDB includes */
#include "monitoring/histogram.h"
Expand Down Expand Up @@ -159,20 +160,20 @@ class explicit_snapshot {
class Rdb_explicit_snapshot : public explicit_snapshot {
public:
static std::shared_ptr<Rdb_explicit_snapshot> create(
snapshot_info_st *ss_info, rocksdb::DB *db,
snapshot_info_st *ssinfo, rocksdb::DB *db,
const rocksdb::Snapshot *snapshot) {
std::lock_guard<std::mutex> lock(explicit_snapshot_mutex);
auto s = std::unique_ptr<rocksdb::ManagedSnapshot>(
new rocksdb::ManagedSnapshot(db, snapshot));
if (!s) {
return nullptr;
}
ss_info->snapshot_id = ++explicit_snapshot_counter;
auto ret = std::make_shared<Rdb_explicit_snapshot>(*ss_info, std::move(s));
ssinfo->snapshot_id = ++explicit_snapshot_counter;
auto ret = std::make_shared<Rdb_explicit_snapshot>(*ssinfo, std::move(s));
if (!ret) {
return nullptr;
}
explicit_snapshots[ss_info->snapshot_id] = ret;
explicit_snapshots[ssinfo->snapshot_id] = ret;
return ret;
}

Expand Down Expand Up @@ -204,9 +205,9 @@ class Rdb_explicit_snapshot : public explicit_snapshot {

rocksdb::ManagedSnapshot *get_snapshot() { return snapshot.get(); }

Rdb_explicit_snapshot(snapshot_info_st ss_info,
Rdb_explicit_snapshot(snapshot_info_st ssinfo,
std::unique_ptr<rocksdb::ManagedSnapshot> &&snapshot)
: explicit_snapshot(ss_info), snapshot(std::move(snapshot)) {}
: explicit_snapshot(ssinfo), snapshot(std::move(snapshot)) {}

virtual ~Rdb_explicit_snapshot() {
std::lock_guard<std::mutex> lock(explicit_snapshot_mutex);
Expand Down Expand Up @@ -927,7 +928,7 @@ static void rocksdb_set_rocksdb_info_log_level(
RDB_MUTEX_LOCK_CHECK(rdb_sysvars_mutex);
rocksdb_info_log_level = *static_cast<const uint64_t *>(save);
rocksdb_db_options->info_log->SetInfoLogLevel(
static_cast<const rocksdb::InfoLogLevel>(rocksdb_info_log_level));
static_cast<rocksdb::InfoLogLevel>(rocksdb_info_log_level));
RDB_MUTEX_UNLOCK_CHECK(rdb_sysvars_mutex);
}

Expand All @@ -939,8 +940,7 @@ static void rocksdb_set_rocksdb_stats_level(

RDB_MUTEX_LOCK_CHECK(rdb_sysvars_mutex);
rocksdb_db_options->statistics->set_stats_level(
static_cast<const rocksdb::StatsLevel>(
*static_cast<const uint64_t *>(save)));
static_cast<rocksdb::StatsLevel>(*static_cast<const uint64_t *>(save)));
// Actual stats level is defined at rocksdb dbopt::statistics::stats_level_
// so adjusting rocksdb_stats_level here to make sure it points to
// the correct stats level.
Expand Down Expand Up @@ -8401,7 +8401,7 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf,
const Rdb_key_def &kd,
bool move_forward) {
int rc = 0;
uint pk_size;
uint pk_size = 0;

/* Get the key columns and primary key value */
const rocksdb::Slice &rkey = m_scan_it->key();
Expand Down Expand Up @@ -9638,8 +9638,8 @@ bool ha_rocksdb::do_bulk_commit(Rdb_transaction *const tx) {
does not contain a primary key. (In which case we generate a hidden
'auto-incremented' pk.)
*/
bool ha_rocksdb::has_hidden_pk(const TABLE *const table) const {
return Rdb_key_def::table_has_hidden_pk(table);
bool ha_rocksdb::has_hidden_pk(const TABLE *const tabl) const {
return Rdb_key_def::table_has_hidden_pk(tabl);
}

/*
Expand Down Expand Up @@ -11480,7 +11480,7 @@ void ha_rocksdb::update_table_stats_if_needed() {
uint64 n_rows = m_tbl_def->m_tbl_stats.m_stat_n_rows;

if (counter > std::max(rocksdb_table_stats_recalc_threshold_count,
static_cast<uint64>(
static_cast<unsigned long long>(
n_rows * rocksdb_table_stats_recalc_threshold_pct /
100.0))) {
// Add the table to the recalc queue
Expand Down
4 changes: 1 addition & 3 deletions storage/rocksdb/ha_rocksdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,7 @@ class ha_rocksdb : public my_core::handler {
HA_PARTIAL_COLUMN_READ | HA_ONLINE_ANALYZE);
}

/* TODO(yzha) - 070a257a1c3 Issue #108: Index-only scans do not work for
* partitioned tables and extended keys */
bool init_with_fields() /* override */;
bool init_with_fields() override;

/** @brief
This is a bitmap of flags that indicates how the storage engine
Expand Down
4 changes: 2 additions & 2 deletions storage/rocksdb/rdb_cf_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class Rdb_cf_options {
const std::string &cf_name);

void get_cf_options(const std::string &cf_name,
rocksdb::ColumnFamilyOptions *const opts
MY_ATTRIBUTE((__nonnull__)));
rocksdb::ColumnFamilyOptions *const opts)
MY_ATTRIBUTE((__nonnull__));

static bool parse_cf_options(const std::string &cf_options,
Name_to_config_t *option_map,
Expand Down
4 changes: 2 additions & 2 deletions storage/rocksdb/rdb_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, uint *offset,
uint field_offset = field->field_ptr() - table->record[0];
*offset = field_offset;
uint null_offset = field->null_offset();
bool maybe_null = field->real_maybe_null();
bool maybe_null = field->is_nullable();
field->move_field(buf + field_offset,
maybe_null ? buf + null_offset : nullptr, field->null_bit);

Expand Down Expand Up @@ -463,7 +463,7 @@ void Rdb_converter::setup_field_encoders() {
m_encoder_arr[i].m_field_index = i;
m_encoder_arr[i].m_pack_length_in_rec = field->pack_length_in_rec();

if (field->real_maybe_null()) {
if (field->is_nullable()) {
m_encoder_arr[i].m_null_mask = cur_null_mask;
m_encoder_arr[i].m_null_offset = null_bytes_length;
if (cur_null_mask == 0x80) {
Expand Down
36 changes: 17 additions & 19 deletions storage/rocksdb/rdb_datadic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ int Rdb_convert_to_record_key_decoder::decode(
uint field_offset = field->field_ptr() - table->record[0];
*offset = field_offset;
uint null_offset = field->null_offset();
bool maybe_null = field->real_maybe_null();
bool maybe_null = field->is_nullable();

field->move_field(buf + field_offset,
maybe_null ? buf + null_offset : nullptr, field->null_bit);
Expand Down Expand Up @@ -362,9 +362,8 @@ Rdb_key_def::Rdb_key_def(const Rdb_key_def &k)
m_total_index_flags_length == 0);
if (k.m_pack_info) {
const size_t size = sizeof(Rdb_field_packing) * k.m_key_parts;
m_pack_info = reinterpret_cast<Rdb_field_packing *>(
my_malloc(PSI_NOT_INSTRUMENTED, size, MYF(0)));
memcpy(m_pack_info, k.m_pack_info, size);
void *buf = my_malloc(PSI_NOT_INSTRUMENTED, size, MYF(0));
m_pack_info = new (buf) Rdb_field_packing(*k.m_pack_info);
}

if (k.m_pk_part_no) {
Expand Down Expand Up @@ -503,7 +502,7 @@ void Rdb_key_def::setup(const TABLE *const tbl,
}
}

if (field && field->real_maybe_null()) max_len += 1; // NULL-byte
if (field && field->is_nullable()) max_len += 1; // NULL-byte

m_pack_info[dst_i].setup(this, field, keyno_to_set, keypart_to_set,
key_part ? key_part->length : 0);
Expand Down Expand Up @@ -550,7 +549,7 @@ void Rdb_key_def::setup(const TABLE *const tbl,
m_ttl_column.c_str())) {
DBUG_ASSERT(field->real_type() == MYSQL_TYPE_LONGLONG);
DBUG_ASSERT(field->key_type() == HA_KEYTYPE_ULONGLONG);
DBUG_ASSERT(!field->real_maybe_null());
DBUG_ASSERT(!field->is_nullable());
m_ttl_pk_key_part_offset = dst_i;
}

Expand Down Expand Up @@ -681,8 +680,7 @@ uint Rdb_key_def::extract_ttl_col(const TABLE *const table_arg,
if (!my_strcasecmp(system_charset_info, field->field_name,
ttl_col_str.c_str()) &&
field->real_type() == MYSQL_TYPE_LONGLONG &&
field->key_type() == HA_KEYTYPE_ULONGLONG &&
!field->real_maybe_null()) {
field->key_type() == HA_KEYTYPE_ULONGLONG && !field->is_nullable()) {
*ttl_column = ttl_col_str;
*ttl_field_index = i;
found = true;
Expand Down Expand Up @@ -1081,12 +1079,12 @@ size_t Rdb_key_def::get_unpack_header_size(char tag) {
*/
void Rdb_key_def::get_lookup_bitmap(const TABLE *table, MY_BITMAP *map) const {
DBUG_ASSERT(map->bitmap == nullptr);
bitmap_init(map, nullptr, MAX_REF_PARTS, false);
bitmap_init(map, nullptr, MAX_REF_PARTS);
uint curr_bitmap_pos = 0;

// Indicates which columns in the read set might be covered.
MY_BITMAP maybe_covered_bitmap;
bitmap_init(&maybe_covered_bitmap, nullptr, table->read_set->n_bits, false);
bitmap_init(&maybe_covered_bitmap, nullptr, table->read_set->n_bits);

for (uint i = 0; i < m_key_parts; i++) {
if (table_has_hidden_pk(table) && i + 1 == m_key_parts) {
Expand Down Expand Up @@ -1170,7 +1168,7 @@ bool Rdb_key_def::covers_lookup(const rocksdb::Slice *const unpack_info,

MY_BITMAP covered_bitmap;
my_bitmap_map covered_bits;
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS, false);
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS);
covered_bits = rdb_netbuf_to_uint16((const uchar *)unpack_header +
sizeof(RDB_UNPACK_COVERED_DATA_TAG));

Expand All @@ -1190,7 +1188,7 @@ uchar *Rdb_key_def::pack_field(
uchar *const packed_tuple MY_ATTRIBUTE((__unused__)),
uchar *const pack_buffer, Rdb_string_writer *const unpack_info,
uint *const n_null_fields) const {
if (field->real_maybe_null()) {
if (field->is_nullable()) {
DBUG_ASSERT(is_storage_available(tuple - packed_tuple, 1));
if (field->is_real_null()) {
/* NULL value. store '\0' so that it sorts before non-NULL values */
Expand Down Expand Up @@ -1306,7 +1304,7 @@ uint Rdb_key_def::pack_record(const TABLE *const tbl, uchar *const pack_buffer,
// Insert TTL timestamp
if (has_ttl() && ttl_bytes) {
write_index_flag_field(unpack_info,
reinterpret_cast<const uchar *const>(ttl_bytes),
reinterpret_cast<const uchar *>(ttl_bytes),
Rdb_key_def::TTL_FLAG);
}
}
Expand All @@ -1332,7 +1330,7 @@ uint Rdb_key_def::pack_record(const TABLE *const tbl, uchar *const pack_buffer,
MY_BITMAP covered_bitmap;
my_bitmap_map covered_bits;
uint curr_bitmap_pos = 0;
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS, false);
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS);

for (uint i = 0; i < n_key_parts; i++) {
// Fill hidden pk id into the last key part for secondary keys for tables
Expand All @@ -1347,7 +1345,7 @@ uint Rdb_key_def::pack_record(const TABLE *const tbl, uchar *const pack_buffer,

uint field_offset = field->field_ptr() - tbl->record[0];
uint null_offset = field->null_offset(tbl->record[0]);
bool maybe_null = field->real_maybe_null();
bool maybe_null = field->is_nullable();

field->move_field(
const_cast<uchar *>(record) + field_offset,
Expand Down Expand Up @@ -1616,7 +1614,7 @@ int Rdb_key_def::unpack_record(TABLE *const table, uchar *const buf,
bool has_covered_bitmap =
has_unpack_info && (unpack_header[0] == RDB_UNPACK_COVERED_DATA_TAG);
if (has_covered_bitmap) {
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS, false);
bitmap_init(&covered_bitmap, &covered_bits, MAX_REF_PARTS);
covered_bits = rdb_netbuf_to_uint16((const uchar *)unpack_header +
sizeof(RDB_UNPACK_COVERED_DATA_TAG));
}
Expand Down Expand Up @@ -2129,7 +2127,7 @@ void Rdb_key_def::pack_double(
float8get(&nr, ptr);
} else {
#endif
doubleget(&nr, ptr);
nr = doubleget(ptr);
#ifdef WORDS_BIGENDIAN
}
#endif
Expand Down Expand Up @@ -2869,7 +2867,7 @@ void Rdb_key_def::store_field(const uchar *data, const size_t length,
auto field_blob = (Field_blob *)field;
auto length_bytes = field_blob->pack_length_no_ptr();
field_blob->store_length(length);
auto blob_data = (char *const)(data);
auto blob_data = (char *)(data);
memset(field_blob->field_ptr() + length_bytes, 0, 8);
memcpy(field_blob->field_ptr() + length_bytes, &blob_data,
sizeof(uchar **));
Expand Down Expand Up @@ -3713,7 +3711,7 @@ bool Rdb_field_packing::setup(const Rdb_key_def *const key_descr,
m_keynr = keynr_arg;
m_key_part = key_part_arg;

m_maybe_null = field ? field->real_maybe_null() : false;
m_maybe_null = field ? field->is_nullable() : false;
m_unpack_func = nullptr;
m_make_unpack_info_func = nullptr;
m_unpack_data_len = 0;
Expand Down
2 changes: 1 addition & 1 deletion storage/rocksdb/rdb_datadic.h
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ extern std::array<const Rdb_collation_codec *, MY_ALL_CHARSETS_SIZE>

class Rdb_field_packing {
public:
Rdb_field_packing(const Rdb_field_packing &) = delete;
Rdb_field_packing(const Rdb_field_packing &o) = default;
Rdb_field_packing &operator=(const Rdb_field_packing &) = delete;
Rdb_field_packing() = default;

Expand Down
2 changes: 1 addition & 1 deletion storage/rocksdb/rdb_index_merge.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class Rdb_index_merge {
ulonglong m_curr_offset; /* offset of the record pointer for the block */
ulonglong m_disk_start_offset; /* where the chunk starts on disk */
ulonglong m_disk_curr_offset; /* current offset on disk */
ulonglong m_total_size; /* total # of data bytes in chunk */
uint64 m_total_size; /* total # of data bytes in chunk */

void store_key_value(const rocksdb::Slice &key, const rocksdb::Slice &val)
MY_ATTRIBUTE((__nonnull__));
Expand Down
2 changes: 1 addition & 1 deletion storage/rocksdb/rdb_threads.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace myrocks {
void *Rdb_thread::thread_func(void *const thread_ptr) {
DBUG_ASSERT(thread_ptr != nullptr);
my_thread_init();
Rdb_thread *const thread = static_cast<Rdb_thread *const>(thread_ptr);
Rdb_thread *const thread = static_cast<Rdb_thread *>(thread_ptr);

if (!thread->m_run_once.exchange(true)) {
thread->setname();
Expand Down

0 comments on commit e132d68

Please sign in to comment.