Remove serialization non C.41 constructors from query condition. (#4684)

This change removes non C.41 constructors from condition_from_capnp.

---
TYPE: NO_HISTORY
DESC: Remove serialization non C.41 constructors from query condition.

---------

Co-authored-by: KiterLuc <[email protected]>

Author: Abigale Kim

test/src/unit-QueryCondition-serialization.cc

@@ -51,7 +51,6 @@ TEST_CASE(
     "QueryCondition serialization: Test serialization",
     "[QueryCondition][serialization]") {
   QueryCondition query_condition;
-  QueryCondition query_condition_clone;
 
   SECTION("Test serialization, basic") {
     std::string field_name = "x";
@@ -271,9 +270,8 @@ TEST_CASE(
   REQUIRE(tiledb::sm::serialization::condition_to_capnp(
               query_condition, &condition_builder)
               .ok());
-  REQUIRE(tiledb::sm::serialization::condition_from_capnp(
-              condition_builder, &query_condition_clone)
-              .ok());
+  auto query_condition_clone =
+      tiledb::sm::serialization::condition_from_capnp(condition_builder);
   REQUIRE(tiledb::test::ast_equal(
       query_condition.ast(), query_condition_clone.ast()));
 }

test/src/unit-cppapi-query-condition-sets.cc

@@ -924,7 +924,7 @@ QueryCondition CPPQueryConditionFx::serialize_deserialize_qc(
   throw_if_not_ok(condition_to_capnp(*qc_ptr, &builder));
 
   // Deserialize the query condition.
-  throw_if_not_ok(condition_from_capnp(builder, ret_ptr));
+  *ret_ptr = condition_from_capnp(builder);
   REQUIRE(tiledb::test::ast_equal(ret_ptr->ast(), qc_ptr->ast()));
 
   return ret;

tiledb/sm/query/query_condition.cc

@@ -2757,10 +2757,6 @@ uint64_t QueryCondition::condition_index() const {
   return condition_index_;
 }
 
-void QueryCondition::set_ast(tdb_unique_ptr<ASTNode>&& ast) {
-  tree_ = std::move(ast);
-}
-
 // Explicit template instantiations.
 template Status QueryCondition::apply_sparse<uint8_t>(
     const ArraySchema& array_schema, ResultTile&, std::vector<uint8_t>&);

tiledb/sm/query/query_condition.h

@@ -59,10 +59,10 @@ class QueryCondition {
   /*     CONSTRUCTORS & DESTRUCTORS    */
   /* ********************************* */
 
-  /** Default constructor. */
+  /** Default constructor. Should be used only in the C API. */
   QueryCondition();
 
-  /** Constructor for a set membership QueryCondition */
+  /** Constructor for a set membership QueryCondition. */
   QueryCondition(
       const std::string& field_name,
       const void* data,
@@ -247,12 +247,6 @@ class QueryCondition {
    */
   QueryCondition negated_condition();
 
-  /**
-   * Sets the AST. This is internal state to only be used in
-   * the serialization path.
-   */
-  void set_ast(tdb_unique_ptr<ASTNode>&& ast);
-
   /**
    * Returns the AST object. This is internal state to only be used in testing
    * and the serialization path.

tiledb/sm/serialization/query.cc

@@ -1068,9 +1068,8 @@ tdb_unique_ptr<ASTNode> condition_ast_from_capnp(
       tdb_new(ASTNodeExpr, std::move(ast_nodes), combination_op));
 }
 
-Status condition_from_capnp(
-    const capnp::Condition::Reader& condition_reader,
-    QueryCondition* const condition) {
+QueryCondition condition_from_capnp(
+    const capnp::Condition::Reader& condition_reader) {
   if (condition_reader.hasClauses()) {  // coming from older API
     // Accumulating the AST value nodes from the clause list.
     std::vector<tdb_unique_ptr<ASTNode>> ast_nodes;
@@ -1104,19 +1103,20 @@ Status condition_from_capnp(
     // Constructing the tree from the list of AST nodes.
     assert(ast_nodes.size() > 0);
     if (ast_nodes.size() == 1) {
-      condition->set_ast(std::move(ast_nodes[0]));
+      return QueryCondition(std::move(ast_nodes[0]));
     } else {
       auto tree_ptr = tdb_unique_ptr<ASTNode>(tdb_new(
           ASTNodeExpr, std::move(ast_nodes), QueryConditionCombinationOp::AND));
-      condition->set_ast(std::move(tree_ptr));
+      return QueryCondition(std::move(tree_ptr));
     }
   } else if (condition_reader.hasTree()) {
     // Constructing the query condition from the AST representation.
     // We assume that the deserialized values of the AST are validated properly.
     auto ast_reader = condition_reader.getTree();
-    condition->set_ast(condition_ast_from_capnp(ast_reader));
+    return QueryCondition(condition_ast_from_capnp(ast_reader));
   }
-  return Status::Ok();
+  throw std::runtime_error(
+      "condition_from_capnp: serialized QC has no tree or clauses.");
 }
 
 Status reader_from_capnp(
@@ -1144,8 +1144,7 @@ Status reader_from_capnp(
   // Query condition
   if (reader_reader.hasCondition()) {
     auto condition_reader = reader_reader.getCondition();
-    QueryCondition condition;
-    RETURN_NOT_OK(condition_from_capnp(condition_reader, &condition));
+    QueryCondition condition = condition_from_capnp(condition_reader);
     RETURN_NOT_OK(query->set_condition(condition));
   }
 
@@ -1186,8 +1185,7 @@ Status index_reader_from_capnp(
   // Query condition
   if (reader_reader.hasCondition()) {
     auto condition_reader = reader_reader.getCondition();
-    QueryCondition condition;
-    RETURN_NOT_OK(condition_from_capnp(condition_reader, &condition));
+    QueryCondition condition = condition_from_capnp(condition_reader);
     RETURN_NOT_OK(query->set_condition(condition));
   }
 
@@ -1229,8 +1227,7 @@ Status dense_reader_from_capnp(
   // Query condition
   if (reader_reader.hasCondition()) {
     auto condition_reader = reader_reader.getCondition();
-    QueryCondition condition;
-    RETURN_NOT_OK(condition_from_capnp(condition_reader, &condition));
+    QueryCondition condition = condition_from_capnp(condition_reader);
     RETURN_NOT_OK(query->set_condition(condition));
   }
 
@@ -1253,8 +1250,7 @@ Status delete_from_capnp(
   // Query condition
   if (delete_reader.hasCondition()) {
     auto condition_reader = delete_reader.getCondition();
-    QueryCondition condition;
-    RETURN_NOT_OK(condition_from_capnp(condition_reader, &condition));
+    QueryCondition condition = condition_from_capnp(condition_reader);
     RETURN_NOT_OK(query->set_condition(condition));
   }
 

tiledb/sm/serialization/query.h

@@ -239,9 +239,8 @@ Status unordered_write_state_from_capnp(
     UnorderedWriter* runordered_writer,
     SerializationContext context);
 
-Status condition_from_capnp(
-    const capnp::Condition::Reader& condition_reader,
-    QueryCondition* const condition);
+QueryCondition condition_from_capnp(
+    const capnp::Condition::Reader& condition_reader);
 
 Status condition_to_capnp(
     const QueryCondition& condition,