Skip to content

Conversation

@BiteTheDDDDt
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions


struct DateTimeFindOp : public CommonFindOp<VecDateTimeValue> {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'find_olap_engine' can be made static [readability-convert-member-functions-to-static]

Suggested change
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
static bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) override {


struct DateFindOp : public CommonFindOp<VecDateTimeValue> {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'find_olap_engine' can be made static [readability-convert-member-functions-to-static]

Suggested change
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
static bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) override {


struct DecimalV2FindOp : public CommonFindOp<DecimalV2Value> {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const {
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'find_olap_engine' can be made static [readability-convert-member-functions-to-static]

Suggested change
bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) const override {
static bool find_olap_engine(const BloomFilterAdaptor& bloom_filter, const void* data) override {


void bitmap_filter_insert_batch(const vectorized::ColumnPtr column,
const std::vector<int>& rows) {
void bitmap_filter_insert_batch(const vectorized::ColumnPtr column, size_t start) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'bitmap_filter_insert_batch' can be made static [readability-convert-member-functions-to-static]

Suggested change
void bitmap_filter_insert_batch(const vectorized::ColumnPtr column, size_t start) {
static void bitmap_filter_insert_batch(const vectorized::ColumnPtr column, size_t start) {

: _build_expr_context(build_expr_ctxs), _runtime_filter_descs(runtime_filter_descs) {}

Status init(RuntimeState* state, int64_t hash_table_size, size_t build_bf_cardinality) {
Status init(RuntimeState* state, int64_t hash_table_size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'init' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status init(RuntimeState* state, int64_t hash_table_size) {
static Status init(RuntimeState* state, int64_t hash_table_size) {

offsets.resize(offsets.size() + indices_end - indices_begin);
auto* dst_offsets_data = offsets.data();

for (auto x = indices_begin; x != indices_end; ++x) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto x' can be declared as 'const auto *x' [readability-qualified-auto]

Suggested change
for (auto x = indices_begin; x != indices_end; ++x) {
for (const auto *x = indices_begin; x != indices_end; ++x) {

auto* dst_data_ptr = chars.data();

size_t dst_chars_pos = old_char_size;
for (auto x = indices_begin; x != indices_end; ++x) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto x' can be declared as 'const auto *x' [readability-qualified-auto]

Suggested change
for (auto x = indices_begin; x != indices_end; ++x) {
for (const auto *x = indices_begin; x != indices_end; ++x) {

}
}

void ColumnStruct::insert_indices_from_join(const IColumn& src, const uint32_t* indices_begin,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: method 'insert_indices_from_join' can be made static [readability-convert-member-functions-to-static]

be/src/vec/columns/column_struct.h:127:

-     void insert_indices_from_join(const IColumn& src, const uint32_t* indices_begin,
+     static void insert_indices_from_join(const IColumn& src, const uint32_t* indices_begin,


#pragma once

#include <gen_cpp/PlanNodes_types.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'gen_cpp/PlanNodes_types.h' file not found [clang-diagnostic-error]

#include <gen_cpp/PlanNodes_types.h>
         ^

using HashMapTable<Key, Cell, Hash, Grower, Allocator>::HashMapTable;

static uint32_t calc_bucket_size(size_t num_elem) {
size_t expect_bucket_size = num_elem + (num_elem - 1) / 7;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 7 is a magic number; consider replacing it with a named constant [readability-magic-numbers]

        size_t expect_bucket_size = num_elem + (num_elem - 1) / 7;
                                                                ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

std::vector<uint8_t> visited;

uint32_t bucket_size = 1;
int max_batch_size = 4064;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 4064 is a magic number; consider replacing it with a named constant [readability-magic-numbers]

    int max_batch_size = 4064;
                         ^

@morningman morningman deleted the new_join branch December 18, 2023 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants