From 2590612061690397b2cec371d646f7907d8549ca Mon Sep 17 00:00:00 2001 From: gal salomon Date: Fri, 1 Jan 2021 11:52:47 +0200 Subject: [PATCH] adding const qualifier to methos/variables; as part of this effort the /reolve_name/ of functions is now done upon semnatic phase before execution; semantic phase should be more the name-resolving(will be on later phase); [ https://github.com/ceph/s3select/pull/42 ] Signed-off-by: gal salomon --- include/s3select.h | 32 +++---- include/s3select_functions.h | 168 +++++++++++++++++++---------------- include/s3select_oper.h | 85 +++++++++--------- 3 files changed, 144 insertions(+), 141 deletions(-) diff --git a/include/s3select.h b/include/s3select.h index 1893868a..706103e9 100644 --- a/include/s3select.h +++ b/include/s3select.h @@ -30,19 +30,6 @@ class s3select_projections std::vector m_projections; public: - bool is_aggregate() - { - //TODO iterate on projections , and search for aggregate - //for(auto p : m_projections){} - - return false; - } - - bool semantic() - { - //TODO check aggragtion function are not nested - return false; - } std::vector* get() { @@ -71,7 +58,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; @@ -80,7 +66,7 @@ struct actionQ actionQ():in_set_count(0), when_than_count(0){} - std::map *> x_map; + std::map *> x_map; ~actionQ() { @@ -88,20 +74,20 @@ struct actionQ delete m.second; } - bool is_already_scanned(void *th,char *a) + bool is_already_scanned(const void *th,const char *a) { //purpose: caller get indication in the case a specific builder is scan more than once the same text(pointer) auto t = x_map.find(th); if(t == x_map.end()) { - auto v = new std::vector;//TODO delete - x_map.insert(std::pair *>(th,v)); + auto v = new std::vector;//TODO delete + x_map.insert(std::pair *>(th,v)); v->push_back(a); } else { - for( auto c : *(t->second) ) + for(auto& c : *(t->second)) { if( strcmp(c,a) == 0) return true; @@ -384,7 +370,9 @@ struct s3select : public bsc::grammar int semantic() { for (const auto &e : get_projections_list()) - {//upon validate there is no aggregation-function nested calls, it validates legit aggregation call. + { + e->resolve_node(); + //upon validate there is no aggregation-function nested calls, it validates legit aggregation call. if (e->is_nested_aggregate(aggr_flow)) { error_description = "nested aggregation function is illegal i.e. sum(...sum ...)"; @@ -410,7 +398,7 @@ struct s3select : public bsc::grammar //in case projection column is not aggregate, the projection column must *not* contain reference to columns. if(e->is_column_reference()) { - error_description = "illegal expression. /select sum(c1) + c1 ..../ is not allow type of query"; + error_description = "illegal query; projection contains aggregation function is not allowed with projection contains column reference"; throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL); } } @@ -1453,7 +1441,7 @@ class csv_object : public base_s3object m_where_clause->traverse_and_apply(m_sa, m_s3_select->get_aliases()); } - for (auto p : m_projections) + for (auto& p : m_projections) { p->traverse_and_apply(m_sa, m_s3_select->get_aliases()); } diff --git a/include/s3select_functions.h b/include/s3select_functions.h index 4fa888ea..03880d9e 100644 --- a/include/s3select_functions.h +++ b/include/s3select_functions.h @@ -146,6 +146,7 @@ class __function : public base_statement base_function* m_func_impl; s3select_functions* m_s3select_functions; variable m_result; + bool m_is_aggregate_function; void _resolve_name() { @@ -160,6 +161,7 @@ class __function : public base_statement throw base_s3select_exception("function not found", base_s3select_exception::s3select_exp_en_t::FATAL); //should abort query } m_func_impl = f; + m_is_aggregate_function= m_func_impl->is_aggregate(); m_s3select_functions->push_for_cleanup(this); //placement new is releasing the main-buffer in which all AST nodes //allocating from it. meaning no calls to destructors. @@ -192,7 +194,7 @@ class __function : public base_statement } } - void set_skip_non_aggregate(bool skip_non_aggregate_op) + void set_skip_non_aggregate(bool skip_non_aggregate_op) override {//it cover the use-case where aggregation function is an argument in non-aggregate function. m_skip_non_aggregate_op = skip_non_aggregate_op; for (auto& ba : arguments) @@ -201,29 +203,31 @@ class __function : public base_statement } } - virtual bool is_aggregate() // TODO under semantic flow + bool is_aggregate() const override { - _resolve_name(); - - return m_func_impl->is_aggregate();//TODO use data-member, to be set upon calling resolve_name() + //_resolve_name(); + //return m_func_impl->is_aggregate(); + + return m_is_aggregate_function; } - virtual bool semantic() + bool semantic() override { return true; } - __function(const char* fname, s3select_functions* s3f) : name(fname), m_func_impl(nullptr), m_s3select_functions(s3f) {} + __function(const char* fname, s3select_functions* s3f) : name(fname), m_func_impl(nullptr), m_s3select_functions(s3f),m_is_aggregate_function(false) + {} - virtual value& eval() + value& eval() override { return eval_internal(); } - virtual value& eval_internal() + value& eval_internal() override { - _resolve_name(); + _resolve_name();//node is "resolved" (function is created) upon first call/first row. if (is_last_call == false) {//all rows prior to last row @@ -252,9 +256,17 @@ class __function : public base_statement return m_result.get_value(); } + void resolve_node() override + { + _resolve_name(); + for (auto& arg : arguments) + { + arg->resolve_node(); + } + } - virtual std::string print(int ident) + std::string print(int ident) override { return std::string(0); } @@ -270,7 +282,7 @@ class __function : public base_statement return arguments; } - virtual ~__function() {} + virtual ~__function() = default; }; @@ -295,9 +307,9 @@ 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; @@ -320,9 +332,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 @@ -357,7 +369,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; @@ -379,9 +391,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(); + auto iter = args->begin(); base_statement *x = *iter; try @@ -417,9 +429,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(); + auto iter = args->begin(); base_statement* x = *iter; if(min > x->eval()) @@ -447,9 +459,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()) @@ -473,7 +485,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; @@ -516,7 +528,7 @@ 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; @@ -607,14 +619,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) @@ -655,9 +667,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) @@ -718,9 +730,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) @@ -790,9 +802,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) @@ -858,7 +870,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(); @@ -879,7 +891,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(); @@ -889,7 +901,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++; @@ -918,7 +930,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; @@ -931,9 +943,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()) { @@ -950,9 +962,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--; @@ -1035,9 +1047,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(); @@ -1062,9 +1074,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(); @@ -1175,9 +1187,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) { @@ -1195,9 +1207,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) { @@ -1216,9 +1228,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) { @@ -1237,9 +1249,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) @@ -1278,11 +1290,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 ++; @@ -1307,7 +1319,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()); @@ -1335,9 +1347,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) { @@ -1360,9 +1372,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(); @@ -1382,9 +1394,9 @@ struct _fn_trim : public base_function { v_remove = " "; } - 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(); base_statement* str = *iter; v_input = str->eval(); @@ -1415,9 +1427,9 @@ struct _fn_leading : public base_function { v_remove = " "; } - 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(); base_statement* str = *iter; v_input = str->eval(); @@ -1447,9 +1459,9 @@ struct _fn_trailing : public base_function { v_remove = " "; } - 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(); base_statement* str = *iter; v_input = str->eval(); @@ -1606,9 +1618,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; } @@ -1618,10 +1630,10 @@ bool base_statement::is_function() } } -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 = 0; if (is_aggregate()) { @@ -1640,9 +1652,9 @@ base_statement* base_statement::get_aggregate() if (is_function()) { - for (auto i : dynamic_cast<__function*>(this)->get_arguments()) + for (auto i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { - base_statement* b=i->get_aggregate(); + const base_statement* b=i->get_aggregate(); if (b) { return b; @@ -1652,7 +1664,7 @@ base_statement* base_statement::get_aggregate() return 0; } -bool base_statement::is_column_reference() +bool base_statement::is_column_reference() const { if(is_column()) return true; @@ -1665,7 +1677,7 @@ bool base_statement::is_column_reference() if(is_function()) { - for(auto a : dynamic_cast<__function*>(this)->get_arguments()) + for(auto a : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { if(a->is_column_reference()) return true; @@ -1675,12 +1687,12 @@ bool base_statement::is_column_reference() return false; } -bool base_statement::is_nested_aggregate(bool &aggr_flow) +bool base_statement::is_nested_aggregate(bool &aggr_flow) const { if (is_aggregate()) { aggr_flow=true; - for (auto& i : dynamic_cast<__function*>(this)->get_arguments()) + for (auto& i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { if (i->get_aggregate() != nullptr) { @@ -1697,7 +1709,7 @@ bool base_statement::is_nested_aggregate(bool &aggr_flow) if (is_function()) { - for (auto& i : dynamic_cast<__function*>(this)->get_arguments()) + for (auto& i : dynamic_cast<__function*>(const_cast(this))->get_arguments()) { if (i->get_aggregate() != nullptr) { diff --git a/include/s3select_oper.h b/include/s3select_oper.h index ee6a65bb..57c077e4 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; @@ -64,7 +64,7 @@ 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) { return (_Allocate(n, (pointer)0)); } @@ -75,7 +75,7 @@ class ChunkAllocator : public std::allocator } //================================== - 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; }; @@ -229,18 +229,8 @@ class scratch_area void update(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) @@ -951,11 +941,11 @@ class base_statement virtual value& eval_internal() = 0; - virtual base_statement* left() + virtual base_statement* left() const { return 0; } - virtual base_statement* right() + virtual base_statement* right() const { return 0; } @@ -990,19 +980,32 @@ 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(); - base_statement* get_aggregate(); - bool is_nested_aggregate(bool&); - bool is_column_reference(); + virtual void resolve_node() + {//part of semantic analysis(TODO maybe semantic method should handle this) + if (left()) + { + left()->resolve_node(); + } + if (right()) + { + right()->resolve_node(); + } + } + + bool is_function() const; + const base_statement* get_aggregate() const; + bool is_nested_aggregate(bool&) const; + bool is_column_reference() const; bool mark_aggreagtion_subtree_to_execute(); virtual void set_last_call() @@ -1184,7 +1187,7 @@ class variable : public base_statement virtual ~variable() {} - virtual bool is_column() //is reference to column. + virtual bool is_column() const //is reference to column. { if(m_var_type == var_t::VAR || m_var_type == var_t::POS) { @@ -1336,11 +1339,11 @@ class arithmetic_operand : public base_statement return true; } - virtual base_statement* left() + base_statement* left() const override { return l; } - virtual base_statement* right() + base_statement* right() const override { return r; } @@ -1418,11 +1421,11 @@ 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; } @@ -1508,11 +1511,11 @@ 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; } @@ -1581,11 +1584,11 @@ 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; } @@ -1654,7 +1657,7 @@ class negate_function_operation : public base_statement return true; } - virtual base_statement* left() + base_statement* left() const override { return function_to_negate; } @@ -1691,13 +1694,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; } virtual void get_aggregate_result(variable*) {} - virtual ~base_function() {} + virtual ~base_function() = default; virtual void dtor() {//release function-body implementation