encode_key to return error when handling nulls

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;
                 }

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
  */

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<char*>(buf_.data()), len};

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<std::string, api::field_type_kind> 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<mock::basic_record> 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");

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<kind::int4, kind::int4, kind::int4>({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<mock::basic_record> result{};
+    execute_query(query, result);
+    ASSERT_EQ(1, result.size());
+    std::sort(result.begin(), result.end());
+    EXPECT_EQ((mock::create_nullable_record<kind::int4, kind::int4, kind::int4>(1, 11, 1)), result[0]);
+}
 }