From 3e79c3fd0900b864a2d7b24ae6fd00f3c3caa4ba Mon Sep 17 00:00:00 2001 From: Ryoji Kurosawa Date: Mon, 11 Nov 2024 18:54:58 +0900 Subject: [PATCH] encode_key to return error when handling nulls --- .../process/impl/ops/details/encode_key.cpp | 9 ++++----- .../process/impl/ops/details/encode_key.h | 6 ++++-- .../executor/process/impl/ops/join_find.cpp | 4 ++++ test/jogasaki/api/api_test.cpp | 18 ++++++++++++++++++ test/jogasaki/api/sql_join_find_test.cpp | 16 ++++++++++++++++ 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/jogasaki/executor/process/impl/ops/details/encode_key.cpp b/src/jogasaki/executor/process/impl/ops/details/encode_key.cpp index 29d5ba5c3..f566356ec 100644 --- a/src/jogasaki/executor/process/impl/ops/details/encode_key.cpp +++ b/src/jogasaki/executor/process/impl/ops/details/encode_key.cpp @@ -68,17 +68,16 @@ status encode_key( //NOLINT(readability-function-cognitive-complexity) message = ss.str(); return status::err_type_mismatch; } + if(a.empty()) { + // creating search key with null value makes no sense because it does not match any entry + return status::err_integrity_constraint_violation; + } kvs::coding_context cctx{}; if (k.nullable_) { if(auto res = kvs::encode_nullable(a, k.type_, k.spec_, cctx, s);res != status::ok) { return res; } } else { - if(a.empty()) { - // log level was lowered temporarily to address issue #939 - VLOG_LP(log_debug) << "Null assigned for non-nullable field."; - return status::err_integrity_constraint_violation; - } if(auto res = kvs::encode(a, k.type_, k.spec_, cctx, s);res != status::ok) { return res; } diff --git a/src/jogasaki/executor/process/impl/ops/details/encode_key.h b/src/jogasaki/executor/process/impl/ops/details/encode_key.h index 12f87fe81..607af8e79 100644 --- a/src/jogasaki/executor/process/impl/ops/details/encode_key.h +++ b/src/jogasaki/executor/process/impl/ops/details/encode_key.h @@ -31,7 +31,7 @@ namespace jogasaki::executor::process::impl::ops::details { /** * @brief evaluate the search key and encode - * @details evaluate the search key and encode it so that it can be used for search + * @details evaluate the search key and encode it (for primary index or secondary index) for search * @param context the request context * @param keys the key fields to be evaluated * @param input_variables the variables to be used for evaluation @@ -40,7 +40,9 @@ namespace jogasaki::executor::process::impl::ops::details { * @param length the length of the encoded key * @param message the error message filled only when the return value is not status::ok and message is available * @return status::ok when successful - * @return status::err_integrity_constraint_violation when evaluation results in null where it is not allowed + * @return status::err_integrity_constraint_violation when evaluation results contain null, with that search + * should not find any entry (this is not necessarily integrity constraint violation, but the status code is left + * unchanged for backward compatibility) * @return status::err_type_mismatch if the type of the evaluated value does not match the expected type * @return status::err_expression_evaluation_failure any other evaluation failure */ diff --git a/src/jogasaki/executor/process/impl/ops/join_find.cpp b/src/jogasaki/executor/process/impl/ops/join_find.cpp index f9d4b3358..69d9e6c6d 100644 --- a/src/jogasaki/executor/process/impl/ops/join_find.cpp +++ b/src/jogasaki/executor/process/impl/ops/join_find.cpp @@ -98,6 +98,10 @@ bool matcher::operator()( res != status::ok) { status_ = res; // TODO handle msg + if (res == status::err_integrity_constraint_violation) { + // null is assigned for find condition. Nothing should match. + status_ = status::not_found; + } return false; } std::string_view key{static_cast(buf_.data()), len}; diff --git a/test/jogasaki/api/api_test.cpp b/test/jogasaki/api/api_test.cpp index a3d3dd7d8..4ca51a810 100644 --- a/test/jogasaki/api/api_test.cpp +++ b/test/jogasaki/api/api_test.cpp @@ -466,6 +466,24 @@ TEST_F(api_test, scan_with_host_variable) { } } +TEST_F(api_test, scan_with_host_variable_with_nulls) { + // verify comparison with null + std::unordered_map variables{}; + variables.emplace("p1", api::field_type_kind::int4); + + execute_statement("create table t (c0 int primary key, c1 int)"); + execute_statement("create index i on t(c1)"); + execute_statement("INSERT INTO t VALUES (0, null),(1, 1)"); + { + auto ps = api::create_parameter_set(); + ps->set_null("p1"); + std::vector result{}; + execute_query("SELECT * FROM t WHERE c1 <= :p1", variables, *ps, result); + ASSERT_EQ(0, result.size()); + } +} + + TEST_F(api_test, join_find_with_key_null) { // test join_find op, key contains null TODO move to join_find op UT rather than using SQL execute_statement("DELETE FROM T0"); diff --git a/test/jogasaki/api/sql_join_find_test.cpp b/test/jogasaki/api/sql_join_find_test.cpp index 67971e665..975174704 100644 --- a/test/jogasaki/api/sql_join_find_test.cpp +++ b/test/jogasaki/api/sql_join_find_test.cpp @@ -238,4 +238,20 @@ TEST_F(sql_join_find_test, left_outer_with_secondary_index) { EXPECT_EQ((mock::create_nullable_record({2, -1, -1}, {false, true, true})), result[2]); } +TEST_F(sql_join_find_test, use_secondary_index_with_null) { + execute_statement("CREATE TABLE t0 (c0 int)"); + execute_statement("INSERT INTO t0 VALUES (null),(1)"); + execute_statement("CREATE TABLE t1 (c0 int primary key, c1 int)"); + execute_statement("CREATE INDEX i1 on t1(c1)"); + execute_statement("INSERT INTO t1 VALUES (10, null),(11,1)"); + + auto query = "SELECT t0.c0, t1.c0, t1.c1 FROM t0 join t1 on t0.c0=t1.c1"; + EXPECT_TRUE(has_join_find(query)); + EXPECT_TRUE(uses_secondary(query)); + std::vector result{}; + execute_query(query, result); + ASSERT_EQ(1, result.size()); + std::sort(result.begin(), result.end()); + EXPECT_EQ((mock::create_nullable_record(1, 11, 1)), result[0]); +} }