Skip to content

Commit

Permalink
bug fix: in the case of constant value on the left side of expression…
Browse files Browse the repository at this point in the history
… (1+some_column/function), the constant-value was modified per each new-row, which cause a wrong calculation; (#37)
  • Loading branch information
galsalomon66 authored Nov 15, 2020
1 parent 36e1c6c commit 81f5e17
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 10 deletions.
26 changes: 16 additions & 10 deletions include/s3select_oper.h
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ class value
return compute<binop_mult>(*this, v);
}

value& operator/(value& v) // TODO handle division by zero
value& operator/(value& v)
{
if (v.is_null() || this->is_null()) {
v.set_nan();
Expand Down Expand Up @@ -1454,14 +1454,15 @@ class mulldiv_operation : public base_statement

public:

enum class muldiv_t {NA, MULL, DIV, POW,MOD} ;
enum class muldiv_t {NA, MULL, DIV, POW, MOD} ;

private:
base_statement* l;
base_statement* r;

muldiv_t _mulldiv;
value var_value;
value tmp_value;

public:

Expand Down Expand Up @@ -1491,19 +1492,23 @@ class mulldiv_operation : public base_statement
switch (_mulldiv)
{
case muldiv_t::MULL:
return var_value = l->eval() * r->eval();
tmp_value = l->eval();
return var_value = tmp_value * r->eval();
break;

case muldiv_t::DIV:
return var_value = l->eval() / r->eval();
tmp_value = l->eval();
return var_value = tmp_value / r->eval();
break;

case muldiv_t::POW:
return var_value = l->eval() ^ r->eval();
tmp_value = l->eval();
return var_value = tmp_value ^ r->eval();
break;

case muldiv_t::MOD:
return var_value = l->eval() % r->eval();
tmp_value = l->eval();
return var_value = tmp_value % r->eval();
break;

default:
Expand All @@ -1530,6 +1535,7 @@ class addsub_operation : public base_statement

addsub_op_t _op;
value var_value;
value tmp_value;

public:

Expand Down Expand Up @@ -1571,12 +1577,12 @@ class addsub_operation : public base_statement
}
}
else if (_op == addsub_op_t::ADD)
{
return var_value = (l->eval() + r->eval());
{tmp_value=l->eval();
return var_value = (tmp_value + r->eval());
}
else
{
return var_value = (l->eval() - r->eval());
{tmp_value=l->eval();
return var_value = (tmp_value - r->eval());
}

return var_value;
Expand Down
37 changes: 37 additions & 0 deletions test/s3select_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,43 @@ TEST(TestS3selectFunctions, max)
ASSERT_EQ(s3select_result, std::string("127,12.699999999999999,"));
}


int count_string(std::string in,std::string substr)
{
int count = 0;
size_t nPos = in.find(substr, 0); // first occurrence
while(nPos != std::string::npos)
{
count++;
nPos = in.find(substr, nPos + 1);
}

return count;
}

TEST(TestS3selectFunctions, binop_constant)
{
//bug-fix for expresion with constant value on the left side(the bug change the constant values between rows)
s3select s3select_syntax;
const std::string input_query = "select 10+1,20-12,2*3,128/2,29%5,2^10 from stdin;";
auto status = s3select_syntax.parse_query(input_query.c_str());
ASSERT_EQ(status, 0);
s3selectEngine::csv_object s3_csv_object(&s3select_syntax);
std::string s3select_result;
std::string input;
size_t size = 128;
generate_csv(input, size);
status = s3_csv_object.run_s3select_on_object(s3select_result, input.c_str(), input.size(),
false, // dont skip first line
false, // dont skip last line
true // aggregate call
);
ASSERT_EQ(status, 0);

int count = count_string(s3select_result,"11,8,6,64,4,1024");
ASSERT_EQ(count,size);
}

TEST(TestS3selectOperator, add)
{
const std::string input_query = "select -5 + 0.5 + -0.25 from stdin;" ;
Expand Down

0 comments on commit 81f5e17

Please sign in to comment.