Skip to content

Commit 1ea37ae

Browse files
authored
The semantic analysis for nested-calls of non-aggregate functions wit… (#46)
* The semantic analysis for nested-calls of non-aggregate functions with aggregate function was wrong, this PR fix several issues. first it rejects illegal queries, second it executes correctly these use-cases. Signed-off-by: gal salomon <gal.salomon@gmail.com> * semantic analysis rejects illegal aggregation queries; it identifying wrong combinations of aggregation projection and non-aggregation projection Signed-off-by: gal salomon <gal.salomon@gmail.com> * marking subtree as runnable; changes to execution flow to support correct execution of nested&complex aggregation queries Signed-off-by: gal salomon <gal.salomon@gmail.com> * adding const qualifier to methods/variables; as part of this effort the /reolve_name/ of functions is now done upon semantic phase before execution; semantic phase should be more the name-resolving(will be on later phase); [ #42 ] Signed-off-by: gal salomon <gal.salomon@gmail.com>
1 parent d723f3f commit 1ea37ae

File tree

4 files changed

+335
-250
lines changed

4 files changed

+335
-250
lines changed

include/s3select.h

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,6 @@ class s3select_projections
3030
std::vector<base_statement*> m_projections;
3131

3232
public:
33-
bool is_aggregate()
34-
{
35-
//TODO iterate on projections , and search for aggregate
36-
//for(auto p : m_projections){}
37-
38-
return false;
39-
}
40-
41-
bool semantic()
42-
{
43-
//TODO check aggragtion function are not nested
44-
return false;
45-
}
4633

4734
std::vector<base_statement*>* get()
4835
{
@@ -71,7 +58,6 @@ struct actionQ
7158
std::vector<std::string> trimTypeQ;
7259
projection_alias alias_map;
7360
std::string from_clause;
74-
std::vector<std::string> schema_columns;
7561
s3select_projections projections;
7662

7763
uint64_t in_set_count;
@@ -80,28 +66,28 @@ struct actionQ
8066

8167
actionQ():in_set_count(0), when_than_count(0){}
8268

83-
std::map<void*,std::vector<char*> *> x_map;
69+
std::map<const void*,std::vector<const char*> *> x_map;
8470

8571
~actionQ()
8672
{
8773
for(auto m : x_map)
8874
delete m.second;
8975
}
9076

91-
bool is_already_scanned(void *th,char *a)
77+
bool is_already_scanned(const void *th,const char *a)
9278
{
9379
//purpose: caller get indication in the case a specific builder is scan more than once the same text(pointer)
9480
auto t = x_map.find(th);
9581

9682
if(t == x_map.end())
9783
{
98-
auto v = new std::vector<char*>;//TODO delete
99-
x_map.insert(std::pair<void*,std::vector<char*> *>(th,v));
84+
auto v = new std::vector<const char*>;//TODO delete
85+
x_map.insert(std::pair<const void*,std::vector<const char*> *>(th,v));
10086
v->push_back(a);
10187
}
10288
else
10389
{
104-
for( auto c : *(t->second) )
90+
for(auto& c : *(t->second))
10591
{
10692
if( strcmp(c,a) == 0)
10793
return true;
@@ -383,34 +369,42 @@ struct s3select : public bsc::grammar<s3select>
383369

384370
int semantic()
385371
{
386-
for (const auto& e : get_projections_list())
372+
for (const auto &e : get_projections_list())
387373
{
388-
base_statement* aggr;
389-
390-
if ((aggr = e->get_aggregate()) != nullptr)
374+
e->resolve_node();
375+
//upon validate there is no aggregation-function nested calls, it validates legit aggregation call.
376+
if (e->is_nested_aggregate(aggr_flow))
391377
{
392-
if (aggr->is_nested_aggregate(aggr))
393-
{
394-
error_description = "nested aggregation function is illegal i.e. sum(...sum ...)";
395-
throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL);
396-
}
397-
398-
aggr_flow = true;
378+
error_description = "nested aggregation function is illegal i.e. sum(...sum ...)";
379+
throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL);
399380
}
400381
}
401382

402383
if (aggr_flow == true)
403-
for (const auto& e : get_projections_list())
384+
{// atleast one projection column contain aggregation function
385+
for (const auto &e : get_projections_list())
404386
{
405-
auto skip_expr = e->get_aggregate();
387+
auto aggregate_expr = e->get_aggregate();
406388

407-
if (e->is_binop_aggregate_and_column(skip_expr))
389+
if (aggregate_expr)
408390
{
409-
error_description = "illegal expression. /select sum(c1) + c1 ..../ is not allow type of query";
410-
throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL);
391+
//per each column, subtree is mark to skip except for the aggregation function subtree.
392+
//for an example: substring( ... , sum() , count() ) :: the substring is mark to skip execution, while sum and count not.
393+
e->set_skip_non_aggregate(true);
394+
e->mark_aggreagtion_subtree_to_execute();
411395
}
396+
else
397+
{
398+
//in case projection column is not aggregate, the projection column must *not* contain reference to columns.
399+
if(e->is_column_reference())
400+
{
401+
error_description = "illegal query; projection contains aggregation function is not allowed with projection contains column reference";
402+
throw base_s3select_exception(error_description, base_s3select_exception::s3select_exp_en_t::FATAL);
403+
}
404+
}
405+
412406
}
413-
407+
}
414408
return 0;
415409
}
416410

@@ -1447,7 +1441,7 @@ class csv_object : public base_s3object
14471441
m_where_clause->traverse_and_apply(m_sa, m_s3_select->get_aliases());
14481442
}
14491443

1450-
for (auto p : m_projections)
1444+
for (auto& p : m_projections)
14511445
{
14521446
p->traverse_and_apply(m_sa, m_s3_select->get_aliases());
14531447
}
@@ -1483,6 +1477,7 @@ class csv_object : public base_s3object
14831477
for (auto& i : m_projections)
14841478
{
14851479
i->set_last_call();
1480+
i->set_skip_non_aggregate(false);//projection column is set to be runnable
14861481
result.append( i->eval().to_string() );
14871482
result.append(",");
14881483
}

0 commit comments

Comments
 (0)