diff --git a/example/generate_rand_csv.c b/example/generate_rand_csv.c index 67d52ada..6f279469 100644 --- a/example/generate_rand_csv.c +++ b/example/generate_rand_csv.c @@ -6,12 +6,11 @@ int main(int argc, char** argv) { if (argc<3) { - printf("%s \n", argv[0]); + printf("%s \n", argv[0]); return -1; } srand(1234); - int line_no=0; for(int i=0; i #include #include +#include #include "s3select_oper.h" #include "s3select_functions.h" #include "s3select_csv_parser.h" @@ -30,7 +31,7 @@ class s3select_projections std::vector m_projections; public: - bool is_aggregate() + bool is_aggregate() const { //TODO iterate on projections , and search for aggregate //for(auto p : m_projections){} @@ -38,9 +39,9 @@ class s3select_projections return false; } - bool semantic() + bool semantic() const { - //TODO check aggragtion function are not nested + //TODO check aggragation function are not nested return false; } @@ -56,8 +57,9 @@ static s3select_reserved_word g_s3select_reserve_word;//read-only struct actionQ { // upon parser is accepting a token (lets say some number), -// it push it into dedicated queue, later those tokens are poped out to build some "higher" contruct (lets say 1 + 2) -// those containers are used only for parsing phase and not for runtime. +// it pushes it into a dedicated queue. These tokens are later popped out to +// build some "higher" construct (1 + 2 for example). +// The containers below are only used during the parsing phase, not for runtime. std::vector muldivQ; std::vector addsubQ; @@ -71,7 +73,6 @@ struct actionQ std::vector trimTypeQ; projection_alias alias_map; std::string from_clause; - std::vector schema_columns; s3select_projections projections; uint64_t in_set_count; @@ -385,11 +386,11 @@ struct s3select : public bsc::grammar { for (const auto& e : get_projections_list()) { - base_statement* aggr; + const base_statement* aggr; if ((aggr = e->get_aggregate()) != nullptr) { - if (aggr->is_nested_aggregate(aggr)) + if (aggr->is_nested_aggregate()) { error_description = "nested aggregation function is illegal i.e. sum(...sum ...)"; throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL); @@ -399,7 +400,7 @@ struct s3select : public bsc::grammar } } - if (aggr_flow == true) + if (aggr_flow) for (const auto& e : get_projections_list()) { auto skip_expr = e->get_aggregate(); @@ -416,7 +417,7 @@ struct s3select : public bsc::grammar int parse_query(const char* input_query) { - if(get_projections_list().empty() == false) + if(!get_projections_list().empty()) { return 0; //already parsed } @@ -762,13 +763,11 @@ void push_mulop::builder(s3select* self, const char* a, const char* b) const } } -void push_addsub_binop::builder(s3select* self, [[maybe_unused]] const char* a,[[maybe_unused]] const char* b) const +void push_addsub_binop::builder(s3select* self, [[maybe_unused]] const char* a, [[maybe_unused]] const char* b) const { - base_statement* l = nullptr, *r = nullptr; - - r = self->getAction()->exprQ.back(); + base_statement* r = self->getAction()->exprQ.back(); self->getAction()->exprQ.pop_back(); - l = self->getAction()->exprQ.back(); + base_statement* l = self->getAction()->exprQ.back(); self->getAction()->exprQ.pop_back(); addsub_operation::addsub_op_t o = self->getAction()->addsubQ.back(); self->getAction()->addsubQ.pop_back(); @@ -778,11 +777,9 @@ void push_addsub_binop::builder(s3select* self, [[maybe_unused]] const char* a,[ void push_mulldiv_binop::builder(s3select* self, [[maybe_unused]] const char* a, [[maybe_unused]] const char* b) const { - base_statement* vl = nullptr, *vr = nullptr; - - vr = self->getAction()->exprQ.back(); + base_statement* vr = self->getAction()->exprQ.back(); self->getAction()->exprQ.pop_back(); - vl = self->getAction()->exprQ.back(); + base_statement* vl = self->getAction()->exprQ.back(); self->getAction()->exprQ.pop_back(); mulldiv_operation::muldiv_t o = self->getAction()->muldivQ.back(); self->getAction()->muldivQ.pop_back(); @@ -832,7 +829,7 @@ void push_function_expr::builder(s3select* self, const char* a, const char* b) c void push_compare_operator::builder(s3select* self, const char* a, const char* b) const { std::string token(a, b); - arithmetic_operand::cmp_t c = arithmetic_operand::cmp_t::NA; + arithmetic_operand::cmp_t c; if (token == "==") { @@ -865,7 +862,7 @@ void push_compare_operator::builder(s3select* self, const char* a, const char* b void push_logical_operator::builder(s3select* self, const char* a, const char* b) const { std::string token(a, b); - logical_operand::oplog_t l = logical_operand::oplog_t::NA; + logical_operand::oplog_t l; if (token == "and") { @@ -899,17 +896,18 @@ void push_arithmetic_predicate::builder(s3select* self, const char* a, const cha void push_logical_predicate::builder(s3select* self, const char* a, const char* b) const { std::string token(a, b); + base_statement* tl = nullptr; + base_statement* tr = nullptr; - base_statement* tl = nullptr, *tr = nullptr; logical_operand::oplog_t oplog = self->getAction()->logical_compareQ.back(); self->getAction()->logical_compareQ.pop_back(); - if (self->getAction()->condQ.empty() == false) + if (!self->getAction()->condQ.empty()) { tr = self->getAction()->condQ.back(); self->getAction()->condQ.pop_back(); } - if (self->getAction()->condQ.empty() == false) + if (!self->getAction()->condQ.empty()) { tl = self->getAction()->condQ.back(); self->getAction()->condQ.pop_back(); @@ -923,9 +921,9 @@ void push_logical_predicate::builder(s3select* self, const char* a, const char* void push_negation::builder(s3select* self, const char* a, const char* b) const { std::string token(a, b); - base_statement* pred = nullptr; + base_statement* pred{nullptr}; - if (self->getAction()->condQ.empty() == false) + if (!self->getAction()->condQ.empty()) { pred = self->getAction()->condQ.back(); self->getAction()->condQ.pop_back(); @@ -985,7 +983,7 @@ void push_alias_projection::builder(s3select* self, const char* a, const char* b //mapping alias name to base-statement bool res = self->getAction()->alias_map.insert_new_entry(alias_name, bs); - if (res == false) + if (!res) { throw base_s3select_exception(std::string("alias <") + alias_name + std::string("> is already been used in query"), base_s3select_exception::s3select_exp_en_t::FATAL); } @@ -1181,16 +1179,16 @@ void push_data_type::builder(s3select* self, const char* a, const char* b) const if(cast_operator("int")) { - self->getAction()->dataTypeQ.push_back("int"); + self->getAction()->dataTypeQ.emplace_back("int"); }else if(cast_operator("float")) { - self->getAction()->dataTypeQ.push_back("float"); + self->getAction()->dataTypeQ.emplace_back("float"); }else if(cast_operator("string")) { - self->getAction()->dataTypeQ.push_back("string"); + self->getAction()->dataTypeQ.emplace_back("string"); }else if(cast_operator("timestamp")) { - self->getAction()->dataTypeQ.push_back("timestamp"); + self->getAction()->dataTypeQ.emplace_back("timestamp"); } } @@ -1311,7 +1309,6 @@ class base_s3object protected: scratch_area* m_sa; - std::string m_obj_name; public: explicit base_s3object(scratch_area* m) : m_sa(m){} @@ -1468,10 +1465,10 @@ class csv_object : public base_s3object int getMatchRow( std::string& result) //TODO virtual ? getResult { - int number_of_tokens = 0; + int number_of_tokens; - if (m_aggr_flow == true) + if (m_aggr_flow) { do { @@ -1503,7 +1500,7 @@ class csv_object : public base_s3object } if (!m_where_clause || m_where_clause->eval().i64() == true) - for (auto i : m_projections) + for (auto& i : m_projections) { i->eval(); } @@ -1546,7 +1543,7 @@ class csv_object : public base_s3object int extract_csv_header_info() { - if (m_csv_defintion.ignore_header_info == true) + if (m_csv_defintion.ignore_header_info) { while(*m_stream && (*m_stream != m_csv_defintion.row_delimiter )) { @@ -1554,7 +1551,7 @@ class csv_object : public base_s3object } m_stream++; } - else if(m_csv_defintion.use_header_info == true) + else if(m_csv_defintion.use_header_info) { size_t num_of_tokens = getNextRow();//TODO validate number of tokens @@ -1632,11 +1629,10 @@ class csv_object : public base_s3object p_obj_chunk--; //scan until end-of previous line in chunk } - u_int32_t skip_last_bytes = (&(csv_stream[stream_length - 1]) - p_obj_chunk); + int32_t skip_last_bytes = (&(csv_stream[stream_length - 1]) - p_obj_chunk); m_last_line.assign(p_obj_chunk + 1, p_obj_chunk + 1 + skip_last_bytes); //save it for next chunk m_previous_line = true;//it means to skip last line - } return run_s3select_on_object(result, csv_stream, stream_length, m_skip_first_line, m_previous_line, (m_processed_bytes >= obj_size)); @@ -1646,14 +1642,12 @@ class csv_object : public base_s3object public: int run_s3select_on_object(std::string& result, const char* csv_stream, size_t stream_length, bool skip_first_line, bool skip_last_line, bool do_aggregate) { - - m_stream = (char*)csv_stream; m_end_stream = (char*)csv_stream + stream_length; m_is_to_aggregate = do_aggregate; m_skip_last_line = skip_last_line; - if(m_extract_csv_header_info == false) + if(!m_extract_csv_header_info) { extract_csv_header_info(); } @@ -1698,6 +1692,6 @@ class csv_object : public base_s3object } }; -};//namespace +} //namespace #endif diff --git a/include/s3select_functions.h b/include/s3select_functions.h index 01fca1e0..c0f42e99 100644 --- a/include/s3select_functions.h +++ b/include/s3select_functions.h @@ -143,11 +143,11 @@ class __function : public base_statement private: bs_stmt_vec_t arguments; std::string name; - base_function* m_func_impl; + base_function* m_func_impl; // todo: separate the object types used when constructing the __function vs when m_func_impl should be fixed s3select_functions* m_s3select_functions; variable m_result; - void _resolve_name() + void _resolve_name() // todo: separate the object types used when constructing the __function vs when m_func_impl should be fixed { if (m_func_impl) { @@ -183,26 +183,25 @@ class __function : public base_statement } } - virtual bool is_aggregate() // TODO under semantic flow + bool is_aggregate() const override// TODO under semantic flow { - _resolve_name(); + const_cast<__function*>(this)->_resolve_name(); return m_func_impl->is_aggregate(); } - virtual bool semantic() + bool semantic() override { return true; } - __function(const char* fname, s3select_functions* s3f) : name(fname), m_func_impl(0), m_s3select_functions(s3f) {} + __function(const char* fname, s3select_functions* s3f) : name(fname), m_func_impl(nullptr), m_s3select_functions(s3f) {} - virtual value& eval() + value& eval() override { + const_cast<__function*>(this)->_resolve_name(); - _resolve_name(); - - if (is_last_call == false) + if (!is_last_call) { (*m_func_impl)(&arguments, &m_result); } @@ -215,10 +214,9 @@ class __function : public base_statement } - - virtual std::string print(int ident) + std::string print(int ident) override { - return std::string(0); + return std::string{}; } void push_argument(base_statement* arg) @@ -227,12 +225,12 @@ class __function : public base_statement } - bs_stmt_vec_t& get_arguments() + const bs_stmt_vec_t& get_arguments() const { return arguments; } - virtual ~__function() {} + virtual ~__function() = default; }; @@ -250,16 +248,16 @@ class __function : public base_statement } /* - s3-select function defintions + s3-select function definitions */ struct _fn_add : public base_function { value var_result; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* x = *iter; iter++; base_statement* y = *iter; @@ -282,9 +280,9 @@ struct _fn_sum : public base_function aggregate = true; } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* x = *iter; try @@ -319,7 +317,7 @@ struct _fn_count : public base_function aggregate=true; } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { count += 1; @@ -341,10 +339,9 @@ struct _fn_avg : public base_function _fn_avg() : sum(0) { aggregate = true; } - bool operator()(bs_stmt_vec_t* args, variable *result) + bool operator()(bs_stmt_vec_t* args, variable *result) override { - bs_stmt_vec_t::iterator iter = args->begin(); - base_statement *x = *iter; + base_statement *x = *(args->begin()); try { @@ -379,10 +376,9 @@ struct _fn_min : public base_function aggregate=true; } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); - base_statement* x = *iter; + base_statement* x = *(args->begin()); if(min > x->eval()) { @@ -409,9 +405,9 @@ struct _fn_max : public base_function aggregate=true; } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* x = *iter; if(max < x->eval()) @@ -435,7 +431,7 @@ struct _fn_to_int : public base_function value var_result; value func_arg; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { char* perr; int64_t i=0; @@ -454,7 +450,7 @@ struct _fn_to_int : public base_function throw base_s3select_exception("characters after int!"); return false; } - } + } else if (func_arg.type == value::value_En_t::FLOAT) { i = func_arg.dbl(); @@ -474,42 +470,43 @@ struct _fn_to_int : public base_function struct _fn_to_float : public base_function { - value var_result; - value v_from; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - char* perr; - double d=0; value v = (*args->begin())->eval(); - if (v.type == value::value_En_t::STRING) + switch (v.type) { + + case value::value_En_t::STRING: { - errno = 0; - d = strtod(v.str(), &perr) ; //TODO check error before constructor - if ((errno == ERANGE && (d == LONG_MAX || d == LONG_MIN)) || (errno != 0 && d == 0)) { + char* pend; + double d = strtod(v.str(), &pend); + if (errno == ERANGE) { throw base_s3select_exception("converted value would fall out of the range of the result type!"); - return false; - } - - if (*perr != '\0') { - throw base_s3select_exception("characters after float!"); - return false; - } - } - else if (v.type == value::value_En_t::FLOAT) - { - d = v.dbl(); + } + if (pend == v.str()) { + // no number found + throw base_s3select_exception("text cannot be converted to a number"); + } + if (*pend) { + throw base_s3select_exception("extra characters after the number"); + } + + var_result = d; } - else - { - d = v.i64(); + break; + + case value::value_En_t::FLOAT: + var_result = v.dbl(); + break; + + default: + var_result = v.i64(); + break; } - var_result = d; *result = var_result; - return true; } @@ -537,7 +534,7 @@ struct _fn_to_timestamp : public base_function value v_str; - bool datetime_validation() + [[nodiscard]] bool datetime_validation() const { //TODO temporary , should check for leap year @@ -569,14 +566,14 @@ struct _fn_to_timestamp : public base_function return true; } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { hr = 0; mn = 0; sc = 0; - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); if (args_size != 1) @@ -595,7 +592,7 @@ struct _fn_to_timestamp : public base_function bsc::parse_info<> info_dig = bsc::parse(v_str.str(), d_yyyymmdd_dig >> *(separator) >> d_time_dig); - if(datetime_validation()==false or !info_dig.full) + if(!datetime_validation() || !info_dig.full) { throw base_s3select_exception("input date-time is illegal"); } @@ -617,9 +614,9 @@ struct _fn_extact_from_timestamp : public base_function value val_date_part; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); if (args_size < 2) @@ -631,7 +628,7 @@ struct _fn_extact_from_timestamp : public base_function val_date_part = date_part->eval();//TODO could be done once? - if(val_date_part.is_string()== false) + if(!val_date_part.is_string()) { throw base_s3select_exception("first parameter should be string"); } @@ -640,7 +637,7 @@ struct _fn_extact_from_timestamp : public base_function base_statement* ts = *iter; - if(ts->eval().is_timestamp()== false) + if(!ts->eval().is_timestamp()) { throw base_s3select_exception("second parameter is not timestamp"); } @@ -680,9 +677,9 @@ struct _fn_diff_timestamp : public base_function value val_dt1; value val_dt2; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); if (args_size < 3) @@ -697,7 +694,7 @@ struct _fn_diff_timestamp : public base_function iter++; base_statement* dt1_param = *iter; val_dt1 = dt1_param->eval(); - if (val_dt1.is_timestamp() == false) + if (!val_dt1.is_timestamp()) { throw base_s3select_exception("second parameter should be timestamp"); } @@ -705,7 +702,7 @@ struct _fn_diff_timestamp : public base_function iter++; base_statement* dt2_param = *iter; val_dt2 = dt2_param->eval(); - if (val_dt2.is_timestamp() == false) + if (!val_dt2.is_timestamp()) { throw base_s3select_exception("third parameter should be timestamp"); } @@ -752,9 +749,9 @@ struct _fn_add_to_timestamp : public base_function value val_quantity; value val_timestamp; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); if (args_size < 3) @@ -765,7 +762,7 @@ struct _fn_add_to_timestamp : public base_function base_statement* date_part = *iter; val_date_part = date_part->eval();//TODO could be done once? - if(val_date_part.is_string()== false) + if(!val_date_part.is_string()) { throw base_s3select_exception("first parameter should be string"); } @@ -774,7 +771,7 @@ struct _fn_add_to_timestamp : public base_function base_statement* quan = *iter; val_quantity = quan->eval(); - if (val_quantity.is_number() == false) + if (!val_quantity.is_number()) { throw base_s3select_exception("second parameter should be number"); //TODO what about double? } @@ -783,7 +780,7 @@ struct _fn_add_to_timestamp : public base_function base_statement* ts = *iter; val_timestamp = ts->eval(); - if(val_timestamp.is_timestamp() == false) + if(!val_timestamp.is_timestamp()) { throw base_s3select_exception("third parameter should be time-stamp"); } @@ -820,7 +817,7 @@ struct _fn_utcnow : public base_function boost::posix_time::ptime now_ptime; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { int args_size = args->size(); @@ -841,7 +838,7 @@ struct _fn_between : public base_function value res; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { int args_size = args->size(); @@ -851,7 +848,7 @@ struct _fn_between : public base_function throw base_s3select_exception("between operates on 3 expressions");//TODO FATAL } - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* second_expr = *iter; iter++; @@ -880,7 +877,7 @@ static char s3select_ver[10]="41.a"; struct _fn_version : public base_function { value val; //TODO use git to generate sha1 - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { val = &s3select_ver[0]; *result = val; @@ -893,9 +890,9 @@ struct _fn_isnull : public base_function value res; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* expr = *iter; value expr_val = expr->eval(); if ( expr_val.is_null()) { @@ -912,9 +909,9 @@ struct _fn_in : public base_function value res; - bool operator()(bs_stmt_vec_t *args, variable *result) + bool operator()(bs_stmt_vec_t *args, variable *result) override { - int args_size = args->size()-1; + int args_size = static_cast(args->size()) - 1; base_statement *main_expr = (*args)[args_size]; value main_expr_val = main_expr->eval(); args_size--; @@ -943,14 +940,14 @@ struct _fn_like : public base_function value res; std::regex compiled_regex; - _fn_like(value s) + explicit _fn_like(value s) { std::string string_value = s.to_string(); transform(string_value); compiled_regex = std::regex(string_value); } - void transform(std::string& s) + static void transform(std::string& s) { std::string::size_type i = 0; while (!s.empty()) @@ -997,9 +994,9 @@ struct _fn_like : public base_function } } - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); iter++; base_statement* main_expr = *iter; value main_expr_val = main_expr->eval(); @@ -1024,9 +1021,9 @@ struct _fn_substr : public base_function value v_from; value v_to; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); @@ -1137,9 +1134,9 @@ struct _fn_charlength : public base_function { value v_str; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* str = *iter; v_str = str->eval(); if(v_str.type != value::value_En_t::STRING) { @@ -1157,9 +1154,9 @@ struct _fn_lower : public base_function { std::string buff; value v_str; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* str = *iter; v_str = str->eval(); if(v_str.type != value::value_En_t::STRING) { @@ -1178,9 +1175,9 @@ struct _fn_upper : public base_function { std::string buff; value v_str; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* str = *iter; v_str = str->eval(); if(v_str.type != value::value_En_t::STRING) { @@ -1199,9 +1196,9 @@ struct _fn_nullif : public base_function { value x; value y; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); int args_size = args->size(); if (args_size != 2) @@ -1240,11 +1237,11 @@ struct _fn_nullif : public base_function { struct _fn_when_than : public base_function { - value when_value,than_value; + value when_value; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* than_expr = *iter; iter ++; @@ -1269,7 +1266,7 @@ struct _fn_case_when_else : public base_function { value when_than_value; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { base_statement* else_expr = *(args->begin()); @@ -1297,9 +1294,9 @@ struct _fn_coalesce : public base_function value res; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter_begin = args->begin(); + auto iter_begin = args->begin(); int args_size = args->size(); while (args_size >= 1) { @@ -1322,9 +1319,9 @@ struct _fn_string : public base_function value res; - bool operator()(bs_stmt_vec_t* args, variable* result) + bool operator()(bs_stmt_vec_t* args, variable* result) override { - bs_stmt_vec_t::iterator iter = args->begin(); + auto iter = args->begin(); base_statement* expr = *iter; value expr_val = expr->eval(); @@ -1432,12 +1429,12 @@ struct _fn_trailing : public base_function { base_function* s3select_functions::create(std::string_view fn_name,const bs_stmt_vec_t &arguments) { - const FunctionLibrary::const_iterator iter = m_functions_library.find(fn_name.data()); + const auto iter = m_functions_library.find(fn_name.data()); if (iter == m_functions_library.end()) { std::string msg; - msg = std::string{fn_name} + " " + " function not found"; + msg = std::string{fn_name} + " function not found"s; throw base_s3select_exception(msg, base_s3select_exception::s3select_exp_en_t::FATAL); } @@ -1568,9 +1565,9 @@ base_function* s3select_functions::create(std::string_view fn_name,const bs_stmt } } -bool base_statement::is_function() +bool base_statement::is_function() const { - if (dynamic_cast<__function*>(this)) + if (dynamic_cast<__function*>(const_cast(this))) { return true; } @@ -1580,27 +1577,27 @@ bool base_statement::is_function() } } -bool base_statement::is_aggregate_exist_in_expression(base_statement* e) //TODO obsolete ? +bool base_statement::is_aggregate_exist_in_expression() const //TODO obsolete ? { - if (e->is_aggregate()) + if (is_aggregate()) { return true; } - if (e->left() && e->left()->is_aggregate_exist_in_expression(e->left())) + if (left() && left()->is_aggregate_exist_in_expression()) { return true; } - if (e->right() && e->right()->is_aggregate_exist_in_expression(e->right())) + if (right() && right()->is_aggregate_exist_in_expression()) { return true; } - if (e->is_function()) + if (is_function()) { - for (auto i : dynamic_cast<__function*>(e)->get_arguments()) - if (e->is_aggregate_exist_in_expression(i)) + for (const auto& i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) + if (i->is_aggregate_exist_in_expression()) { return true; } @@ -1609,64 +1606,63 @@ bool base_statement::is_aggregate_exist_in_expression(base_statement* e) //TODO return false; } -base_statement* base_statement::get_aggregate() +const base_statement* base_statement::get_aggregate() const { //search for aggregation function in AST - base_statement* res = 0; + const base_statement* res; if (is_aggregate()) { return this; } - if (left() && (res=left()->get_aggregate())!=0) + if (left() && (res=left()->get_aggregate())!=nullptr) { return res; } - if (right() && (res=right()->get_aggregate())!=0) + if (right() && (res=right()->get_aggregate())!=nullptr) { return res; } if (is_function()) { - for (auto i : dynamic_cast<__function*>(this)->get_arguments()) + for (const auto& i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { - base_statement* b=i->get_aggregate(); - if (b) + if (auto b = i->get_aggregate(); b) { return b; } } } - return 0; + return nullptr; } -bool base_statement::is_nested_aggregate(base_statement* e) +bool base_statement::is_nested_aggregate() const { //validate for non nested calls for aggregation function, i.e. sum ( min ( )) - if (e->is_aggregate()) + if (is_aggregate()) { - if (e->left()) + if (left()) { - if (e->left()->is_aggregate_exist_in_expression(e->left())) + if (left()->is_aggregate_exist_in_expression()) { return true; } } - else if (e->right()) + else if (right()) { - if (e->right()->is_aggregate_exist_in_expression(e->right())) + if (right()->is_aggregate_exist_in_expression()) { return true; } } - else if (e->is_function()) + else if (is_function()) { - for (auto i : dynamic_cast<__function*>(e)->get_arguments()) + for (const auto& i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { - if (i->is_aggregate_exist_in_expression(i)) + if (i->is_aggregate_exist_in_expression()) { return true; } @@ -1678,7 +1674,7 @@ bool base_statement::is_nested_aggregate(base_statement* e) } // select sum(c2) ... + c1 ... is not allowed. a binary operation with scalar is OK. i.e. select sum() + 1 -bool base_statement::is_binop_aggregate_and_column(base_statement* skip_expression) +bool base_statement::is_binop_aggregate_and_column(const base_statement* skip_expression) const { if (left() && left() != skip_expression) //can traverse to left { @@ -1686,7 +1682,7 @@ bool base_statement::is_binop_aggregate_and_column(base_statement* skip_expressi { return true; } - else if (left()->is_binop_aggregate_and_column(skip_expression) == true) + else if (left()->is_binop_aggregate_and_column(skip_expression)) { return true; } @@ -1698,7 +1694,7 @@ bool base_statement::is_binop_aggregate_and_column(base_statement* skip_expressi { return true; } - else if (right()->is_binop_aggregate_and_column(skip_expression) == true) + else if (right()->is_binop_aggregate_and_column(skip_expression)) { return true; } @@ -1707,7 +1703,7 @@ bool base_statement::is_binop_aggregate_and_column(base_statement* skip_expressi if (this != skip_expression && is_function()) { - __function* f = (dynamic_cast<__function*>(this)); + const __function* f = (dynamic_cast(this)); bs_stmt_vec_t l = f->get_arguments(); for (auto i : l) { @@ -1715,7 +1711,7 @@ bool base_statement::is_binop_aggregate_and_column(base_statement* skip_expressi { return true; } - if (i->is_binop_aggregate_and_column(skip_expression) == true) + if (i->is_binop_aggregate_and_column(skip_expression)) { return true; } diff --git a/include/s3select_oper.h b/include/s3select_oper.h index 8f2df479..efd0114b 100644 --- a/include/s3select_oper.h +++ b/include/s3select_oper.h @@ -6,8 +6,9 @@ #include #include #include -#include -#include +#include +#include +#include #include #include @@ -30,7 +31,6 @@ class ChunkAllocator : public std::allocator public: typedef size_t size_type; typedef T* pointer; - typedef const T* const_pointer; size_t buffer_capacity; char* buffer_ptr; @@ -48,7 +48,7 @@ class ChunkAllocator : public std::allocator { // allocate storage for _Count elements of type T - pointer res = (pointer)(buffer_ptr + buffer_capacity); + auto res = (pointer)(buffer_ptr + buffer_capacity); buffer_capacity+= sizeof(T) * num_of_element; @@ -64,18 +64,18 @@ class ChunkAllocator : public std::allocator } //================================== - inline pointer allocate(size_type n, const void* hint = 0) + inline pointer allocate(size_type n, [[maybe_unused]] const void* hint = 0) // todo remove as both hides non-virtual and unused { return (_Allocate(n, (pointer)0)); } //================================== - inline void deallocate(pointer p, size_type n) + inline void deallocate(pointer p, size_type n) // todo remove as both hides non-virtual and unused { } //================================== - ChunkAllocator() throw() : std::allocator() + ChunkAllocator() noexcept : std::allocator() { // alloc from main-buffer buffer_capacity = 0; @@ -84,7 +84,7 @@ class ChunkAllocator : public std::allocator } //================================== - ChunkAllocator(const ChunkAllocator& other) throw() : std::allocator(other) + ChunkAllocator(const ChunkAllocator& other) noexcept : std::allocator(other) { // copy const buffer_capacity = 0; @@ -92,7 +92,7 @@ class ChunkAllocator : public std::allocator } //================================== - ~ChunkAllocator() throw() + ~ChunkAllocator() noexcept { //do nothing } @@ -120,7 +120,7 @@ class base_s3select_exception public: std::string _msg; - base_s3select_exception(const char* n) : m_severity(s3select_exp_en_t::NONE) + explicit base_s3select_exception(const char* n) : m_severity(s3select_exp_en_t::NONE) { _msg.assign(n); } @@ -143,7 +143,7 @@ class base_s3select_exception return m_severity; } - virtual ~base_s3select_exception() {} + virtual ~base_s3select_exception() = default; }; @@ -224,30 +224,20 @@ class scratch_area void set_column_pos(const char* n, int pos)//TODO use std::string { - m_column_name_pos.push_back( std::pair(n, pos)); + m_column_name_pos.emplace_back(std::pair(n, pos)); } - void update(std::vector& tokens, size_t num_of_tokens) + void update(const std::vector& tokens, size_t num_of_tokens) { - size_t i=0; - for(auto s : tokens) - { - if (i>=num_of_tokens) - { - break; - } - - m_columns[i++] = s; - } - m_upper_bound = i; - + std::copy_n(tokens.begin(), num_of_tokens, m_columns.begin()); + m_upper_bound = num_of_tokens; } int get_column_pos(const char* n) { //done only upon building the AST, not on "runtime" - for( auto iter : m_column_name_pos) + for (const auto& iter : m_column_name_pos) { if (!strcmp(iter.first.c_str(), n)) { @@ -268,7 +258,7 @@ class scratch_area return m_columns[column_pos]; } - int get_num_of_columns() + int get_num_of_columns() const { return m_upper_bound; } @@ -281,26 +271,26 @@ class s3select_reserved_word enum class reserve_word_en_t { NA, - S3S_NULL,//TODO check AWS defintions for reserve words, its a long list , what about functions-names? + S3S_NULL,//TODO check AWS definitions for reserve words, its a long list , what about functions-names? S3S_NAN } ; using reserved_words = std::map; - const reserved_words m_reserved_words= + inline static const reserved_words m_reserved_words= { {"null",reserve_word_en_t::S3S_NULL},{"NULL",reserve_word_en_t::S3S_NULL}, {"nan",reserve_word_en_t::S3S_NAN},{"NaN",reserve_word_en_t::S3S_NAN} }; - bool is_reserved_word(std::string & token) + static bool is_reserved_word(std::string & token) { return m_reserved_words.find(token) != m_reserved_words.end() ; } - reserve_word_en_t get_reserved_word(std::string & token) + static reserve_word_en_t get_reserved_word(std::string & token) // todo consider returning std::optional { - if (is_reserved_word(token)==true) + if (is_reserved_word(token)) { return m_reserved_words.find(token)->second; } @@ -331,11 +321,11 @@ class projection_alias { //purpose: only unique alias names. - for(auto alias: alias_map) + for(const auto& alias: alias_map) { - if(alias.first.compare(alias_name) == 0) + if(alias.first == alias_name) { - return false; //alias name already exist + return false; //alias name already exists } } @@ -345,16 +335,17 @@ class projection_alias return true; } - base_statement* search_alias(std::string alias_name) + // todo consider returning std::optional + base_statement* search_alias(const std::string&& alias_name) const { - for(auto alias: alias_map) + for(const auto& alias: alias_map) { - if(alias.first.compare(alias_name) == 0) + if(alias.first == alias_name) { - return alias.second; //refernce to execution node + return alias.second; //reference to execution node } } - return 0; + return nullptr; } }; @@ -423,7 +414,7 @@ class value { public: - typedef union + typedef union // consider std::variant { int64_t num; char* str;//TODO consider string_view @@ -553,11 +544,11 @@ class value m_to_string.assign( __val.str ); } - return std::string( m_to_string.c_str() ); + return std::string( m_to_string ); } - value& operator=(value& o) + value& operator=(const value& o) { if(o.type == value_En_t::STRING) { @@ -615,17 +606,17 @@ class value return *this; } - int64_t i64() + int64_t i64() const { return __val.num; } - const char* str() + const char* str() const { return __val.str; } - double dbl() + double dbl() const { return __val.dbl; } @@ -635,7 +626,7 @@ class value return __val.timestamp; } - bool operator<(const value& v)//basic compare operator , most itensive runtime operation + bool operator<(const value& v) const//basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -684,7 +675,7 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator>(const value& v) //basic compare operator , most itensive runtime operation + bool operator>(const value& v) const //basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -733,7 +724,7 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator==(const value& v) //basic compare operator , most itensive runtime operation + bool operator==(const value& v) const //basic compare operator , most itensive runtime operation { //TODO NA possible? if (is_string() && v.is_string()) @@ -782,31 +773,32 @@ class value throw base_s3select_exception("operands not of the same type(numeric , string), while comparision"); } - bool operator<=(const value& v) + + bool operator<=(const value& v) const { - if ((is_null() || v.is_null()) || (is_nan() || v.is_nan())) { + if (is_null() || v.is_null() || is_nan() || v.is_nan()) { return false; - } else { - return !(*this>v); - } + } + + return !(*this > v); } - bool operator>=(const value& v) + bool operator>=(const value& v) const { - if ((is_null() || v.is_null()) || (is_nan() || v.is_nan())) { + if (is_null() || v.is_null() || is_nan() || v.is_nan()) { return false; - } else { - return !(*this //conversion rules for arithmetical binary operations @@ -855,7 +847,6 @@ class value if ((l.is_null() || r.is_null()) || (l.is_nan() || r.is_nan())) { l.set_nan(); - return l; } return l; @@ -923,13 +914,13 @@ class base_statement public: base_statement():m_scratch(nullptr), is_last_call(false), m_is_cache_result(false), m_projection_alias(nullptr), m_eval_stack_depth(0) {} virtual value& eval() =0; - virtual base_statement* left() + virtual base_statement* left() const { - return 0; + return nullptr; } - virtual base_statement* right() + virtual base_statement* right() const { - return 0; + return nullptr; } virtual std::string print(int ident) =0;//TODO complete it, one option to use level parametr in interface , virtual bool semantic() =0;//done once , post syntax , traverse all nodes and validate semantics. @@ -948,20 +939,23 @@ class base_statement } } - virtual bool is_aggregate() + virtual bool is_aggregate() const { return false; } - virtual bool is_column() + virtual bool is_column() const { return false; } - bool is_function(); - bool is_aggregate_exist_in_expression(base_statement* e);//TODO obsolete ? - base_statement* get_aggregate(); - bool is_nested_aggregate(base_statement* e); - bool is_binop_aggregate_and_column(base_statement* skip); + bool is_function() const; + + bool is_aggregate_exist_in_expression() const;//TODO obsolete ? + + const base_statement* get_aggregate() const; + + bool is_nested_aggregate() const; + bool is_binop_aggregate_and_column(const base_statement* skip) const; virtual void set_last_call() { @@ -976,7 +970,7 @@ class base_statement } } - bool is_set_last_call() + bool is_set_last_call() const { return is_last_call; } @@ -986,7 +980,7 @@ class base_statement m_is_cache_result = false; } - bool is_result_cached() + bool is_result_cached() const { return m_is_cache_result == true; } @@ -1083,7 +1077,7 @@ class variable : public base_statement } } - variable(s3select_reserved_word::reserve_word_en_t reserve_word) + explicit variable(s3select_reserved_word::reserve_word_en_t reserve_word) { if (reserve_word == s3select_reserved_word::reserve_word_en_t::S3S_NULL) { @@ -1105,9 +1099,10 @@ class variable : public base_statement } } - void operator=(value& v) + variable& operator=(value& v) { var_value = v; + return *this; } void set_value(const char* s) @@ -1195,7 +1190,7 @@ class variable : public base_statement return var_value; } - virtual value& eval() + value& eval() override { if (m_var_type == var_t::COL_VALUE) { @@ -1223,7 +1218,7 @@ class variable : public base_statement //not enter this scope again column_pos = column_alias; - if(m_projection_alias == 0) + if(m_projection_alias == nullptr) { throw base_s3select_exception(std::string("alias {")+_name+std::string("} or column not exist in schema"), base_s3select_exception::s3select_exp_en_t::FATAL); } @@ -1238,7 +1233,7 @@ class variable : public base_statement throw base_s3select_exception("number of calls exceed maximum size, probably a cyclic reference to alias", base_s3select_exception::s3select_exp_en_t::FATAL); } - if (m_projection_alias->is_result_cached() == false) + if (!m_projection_alias->is_result_cached()) { var_value = m_projection_alias->eval(); m_projection_alias->set_result_cache(var_value); @@ -1258,14 +1253,14 @@ class variable : public base_statement return var_value; } - virtual std::string print(int ident) + std::string print(int ident) override { //std::string out = std::string(ident,' ') + std::string("var:") + std::to_string(var_value.__val.num); //return out; return std::string("#");//TBD } - virtual bool semantic() + bool semantic() override { return false; } @@ -1289,28 +1284,28 @@ class arithmetic_operand : public base_statement public: - virtual bool semantic() + bool semantic() override { return true; } - virtual base_statement* left() + base_statement* left() const override { return l; } - virtual base_statement* right() + base_statement* right() const override { return r; } - virtual std::string print(int ident) + std::string print(int ident) override { //std::string out = std::string(ident,' ') + "compare:" += std::to_string(_cmp) + "\n" + l->print(ident-5) +r->print(ident+5); //return out; return std::string("#");//TBD } - virtual value& eval() + value& eval() override { switch (_cmp) @@ -1347,7 +1342,7 @@ class arithmetic_operand : public base_statement arithmetic_operand(base_statement* _l, cmp_t c, base_statement* _r):l(_l), r(_r), _cmp(c),negation_result(false) {} - arithmetic_operand(base_statement* p)//NOT operator + explicit arithmetic_operand(base_statement* p)//NOT operator { l = dynamic_cast(p)->l; r = dynamic_cast(p)->r; @@ -1356,7 +1351,7 @@ class arithmetic_operand : public base_statement negation_result = ! dynamic_cast(p)->negation_result; } - virtual ~arithmetic_operand() {} + virtual ~arithmetic_operand() = default; }; class logical_operand : public base_statement @@ -1376,23 +1371,23 @@ class logical_operand : public base_statement public: - virtual base_statement* left() + base_statement* left() const override { return l; } - virtual base_statement* right() + base_statement* right() const override { return r; } - virtual bool semantic() + bool semantic() override { return true; } logical_operand(base_statement* _l, oplog_t _o, base_statement* _r):l(_l), r(_r), _oplog(_o),negation_result(false) {} - logical_operand(base_statement * p)//NOT operator + explicit logical_operand(base_statement * p)//NOT operator { l = dynamic_cast(p)->l; r = dynamic_cast(p)->r; @@ -1403,13 +1398,14 @@ class logical_operand : public base_statement virtual ~logical_operand() {} - virtual std::string print(int ident) + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "logical_operand:" += std::to_string(_oplog) + "\n" + l->print(ident - 5) + r->print(ident + 5); //return out; return std::string("#");//TBD } - virtual value& eval() + + value& eval() override { bool res; if (!l || !r) @@ -1421,7 +1417,7 @@ class logical_operand : public base_statement { if (a.i64() == false) { - res = false ^ negation_result; + res = negation_result; return var_value = res; } value b = r->eval(); @@ -1435,7 +1431,7 @@ class logical_operand : public base_statement { if (a.i64() == true) { - res = true ^ negation_result; + res = !negation_result; return var_value = res; } value b = r->eval(); @@ -1466,28 +1462,29 @@ class mulldiv_operation : public base_statement public: - virtual base_statement* left() + base_statement* left() const override { return l; } - virtual base_statement* right() + + base_statement* right() const override { return r; } - virtual bool semantic() + bool semantic() override { return true; } - virtual std::string print(int ident) + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "mulldiv_operation:" += std::to_string(_mulldiv) + "\n" + l->print(ident - 5) + r->print(ident + 5); //return out; return std::string("#");//TBD } - virtual value& eval() + value& eval() override { switch (_mulldiv) { @@ -1519,7 +1516,7 @@ class mulldiv_operation : public base_statement mulldiv_operation(base_statement* _l, muldiv_t c, base_statement* _r):l(_l), r(_r), _mulldiv(c) {} - virtual ~mulldiv_operation() {} + virtual ~mulldiv_operation() = default; }; class addsub_operation : public base_statement @@ -1539,31 +1536,32 @@ class addsub_operation : public base_statement public: - virtual base_statement* left() + base_statement* left() const override { return l; } - virtual base_statement* right() + + base_statement* right() const override { return r; } - virtual bool semantic() + bool semantic() override { return true; } addsub_operation(base_statement* _l, addsub_op_t _o, base_statement* _r):l(_l), r(_r), _op(_o) {} - virtual ~addsub_operation() {} + virtual ~addsub_operation() = default; - virtual std::string print(int ident) + std::string print(int ident) override { //std::string out = std::string(ident, ' ') + "addsub_operation:" += std::to_string(_op) + "\n" + l->print(ident - 5) + r->print(ident + 5); return std::string("#");//TBD } - virtual value& eval() + value& eval() override { if (_op == addsub_op_t::NA) // -num , +num , unary-operation on number { @@ -1600,24 +1598,24 @@ class negate_function_operation : public base_statement public: - negate_function_operation(base_statement *f):function_to_negate(f){} + explicit negate_function_operation(base_statement *f):function_to_negate(f){} - virtual std::string print(int ident) + std::string print(int ident) override { return std::string("#");//TBD } - virtual bool semantic() + bool semantic() override { return true; } - virtual base_statement* left() + base_statement* left() const override { return function_to_negate; } - virtual value& eval() + value& eval() override { res = function_to_negate->eval(); @@ -1649,13 +1647,13 @@ class base_function // validate semantic on creation instead on run-time virtual bool operator()(bs_stmt_vec_t* args, variable* result) = 0; base_function() : aggregate(false) {} - bool is_aggregate() + bool is_aggregate() const { - return aggregate == true; + return aggregate; } virtual void get_aggregate_result(variable*) {} - virtual ~base_function() {} + virtual ~base_function() = default; virtual void dtor() {//release function-body implementation @@ -1664,6 +1662,6 @@ class base_function }; -};//namespace +}//namespace #endif diff --git a/test/s3select_test.cpp b/test/s3select_test.cpp index 67b2fb0a..59566664 100644 --- a/test/s3select_test.cpp +++ b/test/s3select_test.cpp @@ -1,7 +1,6 @@ #include "s3select.h" #include "gtest/gtest.h" #include -#include "boost/date_time/gregorian/gregorian.hpp" #include "boost/date_time/posix_time/posix_time.hpp" using namespace s3selectEngine; @@ -102,7 +101,7 @@ class gen_expr std::string generate() { - std::string exp = ""; + std::string exp; open = 0; for (int i = 0; i < 10; i++) @@ -132,7 +131,7 @@ std::string run_s3select(std::string expression) s3_csv_object.run_s3select_on_object(s3select_result, in.c_str(), in.size(), false, false, true); - s3select_result = s3select_result.substr(0, s3select_result.find_first_of(",")); + s3select_result = s3select_result.substr(0, s3select_result.find_first_of(',')); return s3select_result; } @@ -149,7 +148,7 @@ TEST(TestS3SElect, s3select_vs_C) std::string exp = g.generate(); std::string c_result = run_expression_in_C_prog( exp.c_str() ); - char* err=0; + char* err; double c_dbl_res = strtod(c_result.c_str(), &err); std::string input_query = "select " + exp + " from stdin;" ; @@ -167,7 +166,7 @@ TEST(TestS3SElect, s3select_vs_C) TEST(TestS3SElect, ParseQuery) { //TODO syntax issues ? - //TODO error messeges ? + //TODO error messages ? s3select s3select_syntax;