- absl::StatusOr<Field*> Tuple::GetField -> Field* Tuple::GetField

DorosMichal · May 23, 2023 · 2faae15 · 2faae15
1 parent e3b389b
commit 2faae15
Show file tree

Hide file tree

Showing 11 changed files with 24 additions and 30 deletions.
diff --git a/komfydb/common/field.h b/komfydb/common/field.h
@@ -4,8 +4,6 @@
 #include <iostream>
 #include <string>
 
-#include "absl/status/statusor.h"
-
 #include "komfydb/common/type.h"
 #include "komfydb/execution/aggregator.h"
 #include "komfydb/execution/op.h"
@@ -14,7 +12,6 @@ typedef komfydb::execution::Aggregator::AggregateType AggregateType;
 
 namespace {
 
-using komfydb::common::Type;
 using komfydb::execution::Op;
 
 };  // namespace

diff --git a/komfydb/common/tuple.cc b/komfydb/common/tuple.cc
@@ -86,13 +86,7 @@ int Tuple::Size() const {
   return fields.size();
 }
 
-absl::StatusOr<Field*> Tuple::GetField(int i) const {
-  if (fields.size() <= i || i < 0) {
-    return absl::InvalidArgumentError("Index out of range");
-  }
-  if (fields[i] == nullptr) {
-    return absl::InvalidArgumentError("Field not set yet.");
-  }
+Field* Tuple::GetField(int i) const {
   return fields[i].get();
 }
 
@@ -115,6 +109,10 @@ absl::Status Tuple::SetField(int i, std::unique_ptr<Field> f) {
   return absl::OkStatus();
 }
 
+absl::Status Tuple::SetField(int i, Field* f) {
+  return SetField(i, f->CreateCopy());
+}
+
 Tuple::operator std::string() const {
   std::string res = "";
   for (int i = 0; i < fields.size() - 1; i++) {

diff --git a/komfydb/common/tuple.h b/komfydb/common/tuple.h
@@ -37,12 +37,14 @@ class Tuple {
 
   int Size() const;
 
-  absl::StatusOr<Field*> GetField(int i) const;
+  Field* GetField(int i) const;
 
   absl::StatusOr<std::unique_ptr<Field>> ReleaseField(int i);
 
   absl::Status SetField(int i, std::unique_ptr<Field> f);
 
+  absl::Status SetField(int i, Field* f);
+
   operator std::string() const;
 
   friend std::ostream& operator<<(std::ostream& os, const Tuple& tuple);

diff --git a/komfydb/common/tuple_test.cc b/komfydb/common/tuple_test.cc
@@ -18,9 +18,8 @@ TEST(Tuple, StringConversion) {
 
   EXPECT_TRUE(tuple.SetField(0, std::make_unique<IntField>(1)).ok());
 
-  auto f1 = tuple.GetField(0);
-  ASSERT_TRUE(f1.ok());
-  int i = (static_cast<IntField*>(*f1))->GetValue();
+  Field* f1 = tuple.GetField(0);
+  int i = (static_cast<IntField*>(f1))->GetValue();
   EXPECT_EQ(i, 1);
 };
 

diff --git a/komfydb/execution/aggregate.cc b/komfydb/execution/aggregate.cc
@@ -28,8 +28,7 @@ absl::Status FillGroupIdFields(Tuple& group_id,
                                std::vector<Type>& groupby_types,
                                Record* record) {
   for (int i = 0; i < groupby_fields.size(); i++) {
-    ASSIGN_OR_RETURN(Field * field, record->GetField(groupby_fields[i]));
-    RETURN_IF_ERROR(group_id.SetField(i, field->CreateCopy()));
+    RETURN_IF_ERROR(group_id.SetField(i, record->GetField(groupby_fields[i])));
   }
   return absl::OkStatus();
 }
@@ -39,8 +38,8 @@ absl::Status UpdateGroup(AggregateTuple* group,
                          std::vector<AggregateType>& aggregate_types,
                          Record* record) {
   for (int i = 0; i < aggregate_fields.size(); i++) {
-    ASSIGN_OR_RETURN(Field * next_field, record->GetField(aggregate_fields[i]));
-    RETURN_IF_ERROR(group->ApplyAggregate(aggregate_types[i], i, next_field));
+    RETURN_IF_ERROR(group->ApplyAggregate(
+        aggregate_types[i], i, record->GetField(aggregate_fields[i])));
   }
   group->IncremetGroupSize();
   return absl::OkStatus();
@@ -57,8 +56,8 @@ absl::Status InitializeGroup(AggregateTuple* group,
         break;
       }
       default: {
-        ASSIGN_OR_RETURN(Field * field, record->GetField(aggregate_fields[i]));
-        RETURN_IF_ERROR(group->SetField(i, field->CreateCopy()));
+        RETURN_IF_ERROR(
+            group->SetField(i, record->GetField(aggregate_fields[i])));
         break;
       }
     }

diff --git a/komfydb/execution/aggregate_tuple.cc b/komfydb/execution/aggregate_tuple.cc
@@ -95,7 +95,7 @@ absl::Status AggregateTuple::ApplyAggregate(AggregateType aggregate_type, int i,
 absl::Status AggregateTuple::FinalizeAggregates(
     std::vector<AggregateType> aggregate_types) {
   for (int i = 0; i < Size(); i++) {
-    ASSIGN_OR_RETURN(Field * field, GetField(i));
+    Field* field = GetField(i);
     switch (aggregate_types[i]) {
       case AggregateType::AVG: {
         IntField* int_field = static_cast<IntField*>(field);

diff --git a/komfydb/execution/executor.cc b/komfydb/execution/executor.cc
@@ -127,7 +127,7 @@ std::vector<std::string> GetTupleLines(Tuple* tuple, int length,
                                        int column_width) {
   std::vector<std::string> values;
   for (int i = 0; i < length; i++) {
-    Field* field = *tuple->GetField(i);
+    Field* field = tuple->GetField(i);
     values.push_back(std::string(*field));
   }
   return GetLines(values, column_width);
@@ -136,7 +136,7 @@ std::vector<std::string> GetTupleLines(Tuple* tuple, int length,
 void PrintRecord(Record* record, int length, std::ostream& os) {
   os << "(";
   for (int i = 0; i < length; i++) {
-    Field* field = *record->GetField(i);
+    Field* field = record->GetField(i);
 
     switch (field->GetType().GetValue()) {
       case Type::INT: {

diff --git a/komfydb/execution/join_predicate.cc b/komfydb/execution/join_predicate.cc
@@ -10,8 +10,8 @@ JoinPredicate::JoinPredicate(int l_field_idx, Op op, int r_field_idx)
 
 bool JoinPredicate::Filter(Tuple const& l_tuple, Tuple const& r_tuple) {
   // we assume that if we got to this place all the values are correct
-  Field* l_field = l_tuple.GetField(l_field_idx).value();
-  Field* r_field = r_tuple.GetField(r_field_idx).value();
+  Field* l_field = l_tuple.GetField(l_field_idx);
+  Field* r_field = r_tuple.GetField(r_field_idx);
   return l_field->Compare(op, r_field);
 }
 

diff --git a/komfydb/execution/predicate.cc b/komfydb/execution/predicate.cc
@@ -46,11 +46,10 @@ bool Predicate::Evaluate(const Record& record) {
   /* We assume that GetField() or Compare() cannot fail here */
   switch (type) {
     case Type::COL_COL: {
-      return (*record.GetField(l_field))
-          ->Compare(op, *record.GetField(r_field));
+      return record.GetField(l_field)->Compare(op, record.GetField(r_field));
     }
     case Type::COL_CONST: {
-      return (*record.GetField(l_field))->Compare(op, const_field.get());
+      return record.GetField(l_field)->Compare(op, const_field.get());
     }
   }
 }

diff --git a/komfydb/execution/project.cc b/komfydb/execution/project.cc
@@ -55,7 +55,7 @@ absl::Status Project::FetchNext() {
   int num_of_fields = tuple_desc.Length();
   Tuple new_tuple(num_of_fields);
   for (int i = 0; i < num_of_fields; i++) {
-    ASSIGN_OR_RETURN(Field * record_field, record->GetField(out_field_idxs[i]));
+    Field* record_field = record->GetField(out_field_idxs[i]);
     if (record_field == nullptr) {
       continue;
     }

diff --git a/komfydb/storage/heap_page.cc b/komfydb/storage/heap_page.cc
@@ -183,7 +183,7 @@ absl::StatusOr<std::vector<uint8_t>> HeapPage::GetPageData() {
 
     for (int j = 0; j < tuple_len; j++) {
       ASSIGN_OR_RETURN(Type field_type, tuple_desc->GetFieldType(j));
-      ASSIGN_OR_RETURN(Field * field, record.GetField(j));
+      Field* field = record.GetField(j);
 
       switch (field_type.GetValue()) {
         case Type::STRING: {