Skip to content

Commit

Permalink
fix unknown value evaluation when anding false or oring true
Browse files Browse the repository at this point in the history
  • Loading branch information
@kuron99
kuron99 committed Nov 6, 2024
1 parent cf6b9b3 commit 500f895
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 48 deletions.
69 changes: 49 additions & 20 deletions src/jogasaki/executor/expr/evaluator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,30 +320,56 @@ any engine::remainder_any(any const& left, any const& right) {
}
}

template <class T, class U = T>
any conditional_and(T const& l, U const& r) {
return any{std::in_place_type<T>, l && r};
}

any engine::conditional_and_any(any const& left, any const& right) {
BOOST_ASSERT(left && right); //NOLINT
switch(left.type_index()) {
case any::index<bool>: return conditional_and(left.to<bool>(), right.to<bool>());
default: return return_unsupported();
// first, check if either of operands is false because then
// the result is false regardless of the other operand being true or null
if(left) {
if(left.type_index() != any::index<bool>) {
return return_unsupported();
}
if(! left.to<bool>()) {
return left;
}
}
}

template <class T, class U = T>
any conditional_or(T const& l, U const& r) {
return any{std::in_place_type<T>, l || r};
if(right) {
if(right.type_index() != any::index<bool>) {
return return_unsupported();
}
if(! right.to<bool>()) {
return right;
}
}
// left/right are either true or null
if(! left || ! right) {
return {};
}
return any{std::in_place_type<bool>, true};
}

any engine::conditional_or_any(any const& left, any const& right) {
BOOST_ASSERT(left && right); //NOLINT
switch(left.type_index()) {
case any::index<bool>: return conditional_or(left.to<bool>(), right.to<bool>());
default: return return_unsupported();
// first, check if either of operands is true because then
// the result is true regardless of the other operand being false or null
if(left) {
if(left.type_index() != any::index<bool>) {
return return_unsupported();
}
if(left.to<bool>()) {
return left;
}
}
if(right) {
if(right.type_index() != any::index<bool>) {
return return_unsupported();
}
if(right.to<bool>()) {
return right;
}
}
// left/right are either false or null
if(! left || ! right) {
return {};
}
return any{std::in_place_type<bool>, false};
}

any engine::operator()(takatori::scalar::binary const& exp) {
Expand All @@ -352,8 +378,11 @@ any engine::operator()(takatori::scalar::binary const& exp) {
auto r = dispatch(*this, exp.right());
if (l.error()) return l;
if (r.error()) return r;
if (! l) return l;
if (! r) return r;
if (exp.operator_kind() != optype::conditional_and && exp.operator_kind() != optype::conditional_or) {
// except AND/OR, if either of operands is null, the result is null
if (! l) return l;
if (! r) return r;
}
switch(exp.operator_kind()) {
case optype::add: return add_any(l, r);
case optype::concat: return concat_any(l, r);
Expand Down
20 changes: 20 additions & 0 deletions test/jogasaki/api/sql_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,4 +667,24 @@ TEST_F(sql_test, type_of_cast_conversion) {
EXPECT_EQ((typed_nullable_record<kind::decimal>(std::tuple{meta::decimal_type(5,2)}, std::forward_as_tuple(triple{1, 0, 1, 0}))), result[0]);
}
}

TEST_F(sql_test, conditional_and_or) {
// regression test for issue #1012
execute_statement("create table t (c0 int, c1 int)");
execute_statement("INSERT INTO t (c0) VALUES (1)");
{
// unknown or true
std::vector<mock::basic_record> result{};
execute_query("select c0 from t where c0 < c1 or c1 is null", result);
ASSERT_EQ(1, result.size());
EXPECT_EQ((create_nullable_record<kind::int4>(1)), result[0]);
}
{
// unknown and false
std::vector<mock::basic_record> result{};
execute_query("select c0 from t where c0 < c1 and c1 is not null", result);
ASSERT_EQ(0, result.size());
}
}

}
115 changes: 87 additions & 28 deletions test/jogasaki/executor/process/expression_evaluator_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,14 @@ class expression_evaluator_test : public test_root {
}

template <class In1, class In2, class Out, class Optype, class ... Args>
void test_two_arity_exp(
void test_two_arity_exp_with_null(
Optype op,
typename meta::field_type_traits<utils::to_field_type_kind<In1>()>::runtime_type c1,
bool c1_is_null,
typename meta::field_type_traits<utils::to_field_type_kind<In2>()>::runtime_type c2,
bool c2_is_null,
typename meta::field_type_traits<utils::to_field_type_kind<Out>()>::runtime_type exp,
bool exp_is_null,
Args...args
) {
using T = from_operator_enum<Optype>;
Expand All @@ -223,30 +226,35 @@ class expression_evaluator_test : public test_root {
>;
auto expr = create_two_arity_exp<In1, In2, Out, Optype, Args...>(op, args...);
{
set_values<In1, In2>(c1, c2, false, false);
utils::checkpoint_holder cph{&resource_};
evaluator_context c{&resource_};
auto result = evaluator_(c, vars_, &resource_).to<out_type>();
ASSERT_EQ(exp, result);
}
{
set_values<In1, In2>(c1, c2, true, false);
set_values<In1, In2>(c1, c2, c1_is_null, c2_is_null);
utils::checkpoint_holder cph{&resource_};
evaluator_context c{&resource_};
auto result = evaluator_(c, vars_, &resource_);
ASSERT_TRUE(result.empty());
ASSERT_FALSE(result.error());
}
{
set_values<In1, In2>(c1, c2, false, true);
utils::checkpoint_holder cph{&resource_};
evaluator_context c{&resource_};
auto result = evaluator_(c, vars_, &resource_);
ASSERT_TRUE(result.empty());
ASSERT_FALSE(result.error());
auto a = evaluator_(c, vars_, &resource_);
ASSERT_TRUE(! a.error());
if(exp_is_null) {
ASSERT_TRUE(a.empty());
} else {
ASSERT_TRUE(! a.empty());
auto result = a.to<out_type>();
ASSERT_EQ(exp, result);
}
}
expressions().clear();
}

template <class In1, class In2, class Out, class Optype, class ... Args>
void test_two_arity_exp(
Optype op,
typename meta::field_type_traits<utils::to_field_type_kind<In1>()>::runtime_type c1,
typename meta::field_type_traits<utils::to_field_type_kind<In2>()>::runtime_type c2,
typename meta::field_type_traits<utils::to_field_type_kind<Out>()>::runtime_type exp,
Args...args
) {
test_two_arity_exp_with_null<In1, In2, Out, Optype, Args...>(op, c1, false, c2, false, exp, false, args...);
test_two_arity_exp_with_null<In1, In2, Out, Optype, Args...>(op, c1, true, c2, false, exp, true, args...);
test_two_arity_exp_with_null<In1, In2, Out, Optype, Args...>(op, c1, false, c2, true, exp, true, args...);
}

template<class T>
void test_compare();

Expand Down Expand Up @@ -559,15 +567,66 @@ TEST_F(expression_evaluator_test, compare_time_point) {
compare_time_points(comparison_operator::greater_equal, one, two, false);
}

TEST_F(expression_evaluator_test, conditional_and_or) {
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 1, 1, true);
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 1, 0, false);
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 0, 1, false);
TEST_F(expression_evaluator_test, conditional_and) {
// condiation_and and conditional_or are exceptional operation in that the result is not always null even if one of the operad is null

// T and T = T
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 1, false, 1, false, true, false);

// T and F = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 1, false, 0, false, false, false);

// F and T = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 0, false, 1, false, false, false);

// F and F = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 0, false, 0, false, false, false);

// null and T = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, -1, true, 1, false, false, true);

// T and null = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 1, false, -1, true, false, true);

// null and F = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, -1, true, 0, false, false, false);

// F and null = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, 0, false, -1, true, false, false);

// null and null = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_and, -1, true, -1, true, false, true);
}

TEST_F(expression_evaluator_test, conditional_or) {
// condiation_and and conditional_or are exceptional operation in that the result is not always null even if one of the operad is null

// T or T = T
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 1, false, 1, false, true, false);

// T or F = T
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 1, false, 0, false, true, false);

// F or T = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 0, false, 1, false, true, false);

// F or F = F
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 0, false, 0, false, false, false);

// null or T = T
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, -1, true, 1, false, true, false);

// T or null = T
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 1, false, -1, true, true, false);

// null or F = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, -1, true, 0, false, false, true);

// F or null = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 0, false, -1, true, false, true);

test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 1, 1, true);
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 1, 0, true);
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 0, 1, true);
test_two_arity_exp<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, 0, 0, false);
// null or null = null
test_two_arity_exp_with_null<t::boolean, t::boolean, t::boolean>(binary_operator::conditional_or, -1, true, -1, true, false, true);
}

TEST_F(expression_evaluator_test, arithmetic_error) {
Expand Down

0 comments on commit 500f895

Please sign in to comment.