kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: client: convert the has_foo booleans into boost::optional
Date Sun, 31 Mar 2019 18:48:03 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 13d596f  client: convert the has_foo booleans into boost::optional
13d596f is described below

commit 13d596f86efe322e62efde0f41efed1ae3eebacc
Author: helifu <hzhelifu@corp.netease.com>
AuthorDate: Sun Mar 31 16:32:14 2019 +0800

    client: convert the has_foo booleans into boost::optional
    
    this patch converts the has_foo booleans into boost::optional
    in schema related files in client module, and it can help to
    make the code look cleaner.
    
    Change-Id: Ia4e7319c617bd388730d6e81378b37eb6793cf24
    Reviewed-on: http://gerrit.cloudera.org:8080/12891
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <adar@cloudera.com>
---
 src/kudu/client/schema-internal.h         |  60 +++---------
 src/kudu/client/schema.cc                 | 157 +++++++++++++-----------------
 src/kudu/client/table_alterer-internal.cc |  24 +++--
 3 files changed, 91 insertions(+), 150 deletions(-)

diff --git a/src/kudu/client/schema-internal.h b/src/kudu/client/schema-internal.h
index 923ea35..5ff642e 100644
--- a/src/kudu/client/schema-internal.h
+++ b/src/kudu/client/schema-internal.h
@@ -60,61 +60,29 @@ class KuduColumnSpec::Data {
  public:
   explicit Data(std::string name)
       : name(std::move(name)),
-        has_type(false),
-        has_precision(false),
-        precision(-1),
-        has_scale(false),
-        scale(-1),
-        has_encoding(false),
-        has_compression(false),
-        has_block_size(false),
-        has_nullable(false),
         primary_key(false),
-        has_default(false),
-        default_val(NULL),
-        remove_default(false),
-        has_rename_to(false) {
+        remove_default(false) {
   }
 
   ~Data() {
-    delete default_val;
+    if (default_val) {
+      delete default_val.value();
+    }
   }
 
   const std::string name;
 
-  bool has_type;
-  KuduColumnSchema::DataType type;
-
-  bool has_precision;
-  int8_t precision;
-
-  bool has_scale;
-  int8_t scale;
-
-  bool has_encoding;
-  KuduColumnStorageAttributes::EncodingType encoding;
-
-  bool has_compression;
-  KuduColumnStorageAttributes::CompressionType compression;
-
-  bool has_block_size;
-  int32_t block_size;
-
-  bool has_nullable;
-  bool nullable;
-
+  boost::optional<KuduColumnSchema::DataType> type;
+  boost::optional<int8_t> precision;
+  boost::optional<int8_t> scale;
+  boost::optional<KuduColumnStorageAttributes::EncodingType> encoding;
+  boost::optional<KuduColumnStorageAttributes::CompressionType> compression;
+  boost::optional<int32_t> block_size;
+  boost::optional<bool> nullable;
   bool primary_key;
-
-  bool has_default;
-  KuduValue* default_val; // Owned.
-
-  // For ALTER
-  bool remove_default;
-
-  // For ALTER
-  bool has_rename_to;
-  std::string rename_to;
-
+  boost::optional<KuduValue*> default_val;  // Owned.
+  bool remove_default;                      // For ALTER
+  boost::optional<std::string> rename_to;   // For ALTER
   boost::optional<std::string> comment;
 };
 
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index 33bbab7..faa7c53 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -208,8 +208,8 @@ int8_t KuduColumnTypeAttributes::scale() const {
 // KuduColumnSpec
 ////////////////////////////////////////////////////////////
 
-KuduColumnSpec::KuduColumnSpec(const std::string& name)
-  : data_(new Data(name)) {
+KuduColumnSpec::KuduColumnSpec(const string& col_name)
+  : data_(new Data(col_name)) {
 }
 
 KuduColumnSpec::~KuduColumnSpec() {
@@ -217,46 +217,42 @@ KuduColumnSpec::~KuduColumnSpec() {
 }
 
 KuduColumnSpec* KuduColumnSpec::Type(KuduColumnSchema::DataType type) {
-  data_->has_type = true;
   data_->type = type;
   return this;
 }
 
-KuduColumnSpec* KuduColumnSpec::Default(KuduValue* v) {
-  data_->has_default = true;
-  delete data_->default_val;
-  data_->default_val = v;
+KuduColumnSpec* KuduColumnSpec::Default(KuduValue* value) {
+  if (value == nullptr) return this;
+  if (data_->default_val) {
+    delete data_->default_val.value();
+  }
+  data_->default_val = value;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Compression(
     KuduColumnStorageAttributes::CompressionType compression) {
-  data_->has_compression = true;
   data_->compression = compression;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Encoding(
     KuduColumnStorageAttributes::EncodingType encoding) {
-  data_->has_encoding = true;
   data_->encoding = encoding;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::BlockSize(int32_t block_size) {
-  data_->has_block_size = true;
   data_->block_size = block_size;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Precision(int8_t precision) {
-  data_->has_precision = true;
   data_->precision = precision;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Scale(int8_t scale) {
-  data_->has_scale = true;
   data_->scale = scale;
   return this;
 }
@@ -267,13 +263,11 @@ KuduColumnSpec* KuduColumnSpec::PrimaryKey() {
 }
 
 KuduColumnSpec* KuduColumnSpec::NotNull() {
-  data_->has_nullable = true;
   data_->nullable = false;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Nullable() {
-  data_->has_nullable = true;
   data_->nullable = true;
   return this;
 }
@@ -283,21 +277,20 @@ KuduColumnSpec* KuduColumnSpec::RemoveDefault() {
   return this;
 }
 
-KuduColumnSpec* KuduColumnSpec::RenameTo(const std::string& new_name) {
-  data_->has_rename_to = true;
+KuduColumnSpec* KuduColumnSpec::RenameTo(const string& new_name) {
   data_->rename_to = new_name;
   return this;
 }
 
 KuduColumnSpec* KuduColumnSpec::Comment(const string& comment) {
-  data_->comment = boost::optional<string>(comment);
+  data_->comment = comment;
   return this;
 }
 
 Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const {
   // Verify that the user isn't trying to use any methods that
   // don't make sense for CREATE.
-  if (data_->has_rename_to) {
+  if (data_->rename_to) {
     return Status::NotSupported("cannot rename a column during CreateTable",
                                 data_->name);
   }
@@ -306,80 +299,73 @@ Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const {
                                 data_->name);
   }
 
-  if (!data_->has_type) {
+  if (!data_->type) {
     return Status::InvalidArgument("no type provided for column", data_->name);
   }
 
-  if (data_->type == KuduColumnSchema::DECIMAL) {
-    if (!data_->has_precision) {
+  if (data_->type.value() == KuduColumnSchema::DECIMAL) {
+    if (!data_->precision) {
       return Status::InvalidArgument("no precision provided for decimal column", data_->name);
     }
-    if (data_->precision < kMinDecimalPrecision ||
-        data_->precision > kMaxDecimalPrecision) {
+    if (data_->precision.value() < kMinDecimalPrecision ||
+        data_->precision.value() > kMaxDecimalPrecision) {
       return Status::InvalidArgument(
           strings::Substitute("precision must be between $0 and $1",
                               kMinDecimalPrecision,
                               kMaxDecimalPrecision), data_->name);
     }
-    if (data_->has_scale) {
-      if (data_->scale < kMinDecimalScale) {
+    if (data_->scale) {
+      if (data_->scale.value() < kMinDecimalScale) {
         return Status::InvalidArgument(
             strings::Substitute("scale is less than the minimum value of $0",
                                 kMinDecimalScale), data_->name);
       }
-      if (data_->scale > data_->precision) {
+      if (data_->scale.value() > data_->precision.value()) {
         return Status::InvalidArgument(
             strings::Substitute("scale is greater than the precision value of",
-                                data_->precision), data_->name);
+                                data_->precision.value()), data_->name);
       }
     }
   } else {
-    if (data_->has_precision) {
+    if (data_->precision) {
       return Status::InvalidArgument(
-          strings::Substitute("precision is not valid on a $0 column", data_->type), data_->name);
+          strings::Substitute("precision is not valid on a $0 column",
+                              data_->type.value()), data_->name);
     }
-    if (data_->has_scale) {
+    if (data_->scale) {
       return Status::InvalidArgument(
-          strings::Substitute("scale is not valid on a $0 column", data_->type), data_->name);
+          strings::Substitute("scale is not valid on a $0 column",
+                              data_->type.value()), data_->name);
     }
   }
 
-  int8_t precision = (data_->has_precision) ? data_->precision : 0;
-  int8_t scale = (data_->has_scale) ? data_->scale : kDefaultDecimalScale;
+  int8_t precision = (data_->precision) ? data_->precision.value() : 0;
+  int8_t scale = (data_->scale) ? data_->scale.value() : kDefaultDecimalScale;
 
   KuduColumnTypeAttributes type_attrs(precision, scale);
-  DataType internal_type = ToInternalDataType(data_->type, type_attrs);
-  bool nullable = data_->has_nullable ? data_->nullable : true;
+  DataType internal_type = ToInternalDataType(data_->type.value(), type_attrs);
+  bool nullable = data_->nullable ? data_->nullable.value() : true;
 
   void* default_val = nullptr;
-  // TODO: distinguish between DEFAULT NULL and no default?
-  if (data_->has_default) {
+  // TODO(unknown): distinguish between DEFAULT NULL and no default?
+  if (data_->default_val) {
     ColumnTypeAttributes internal_type_attrs(precision, scale);
-    RETURN_NOT_OK(data_->default_val->data_->CheckTypeAndGetPointer(
+    RETURN_NOT_OK(data_->default_val.value()->data_->CheckTypeAndGetPointer(
         data_->name, internal_type, internal_type_attrs,  &default_val));
   }
 
-  // Encoding and compression
-  KuduColumnStorageAttributes::EncodingType encoding =
-    KuduColumnStorageAttributes::AUTO_ENCODING;
-  if (data_->has_encoding) {
-    encoding = data_->encoding;
-  }
+  // Encoding and compression.
+  KuduColumnStorageAttributes::EncodingType encoding = data_->encoding ?
+      data_->encoding.value() : KuduColumnStorageAttributes::AUTO_ENCODING;
+  KuduColumnStorageAttributes::CompressionType compression = data_->compression ?
+      data_->compression.value() : KuduColumnStorageAttributes::DEFAULT_COMPRESSION;
 
-  KuduColumnStorageAttributes::CompressionType compression =
-    KuduColumnStorageAttributes::DEFAULT_COMPRESSION;
-  if (data_->has_compression) {
-    compression = data_->compression;
-  }
-
-  int32_t block_size = 0; // '0' signifies server-side default
-  if (data_->has_block_size) {
-    block_size = data_->block_size;
-  }
+  // BlockSize: '0' signifies server-side default.
+  int32_t block_size = data_->block_size ? data_->block_size.value() : 0;
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-  *col = KuduColumnSchema(data_->name, data_->type, nullable,
+  *col = KuduColumnSchema(data_->name, data_->type.value(), nullable,
                           default_val,
                           KuduColumnStorageAttributes(encoding, compression, block_size),
                           type_attrs, data_->comment.get_ptr());
@@ -389,55 +375,47 @@ Status KuduColumnSpec::ToColumnSchema(KuduColumnSchema* col) const {
 }
 
 Status KuduColumnSpec::ToColumnSchemaDelta(ColumnSchemaDelta* col_delta) const {
-  if (data_->has_type) {
+  if (data_->type) {
     return Status::InvalidArgument("type provided for column schema delta", data_->name);
   }
-  if (data_->has_nullable) {
+  if (data_->nullable) {
     return Status::InvalidArgument("nullability provided for column schema delta", data_->name);
   }
   if (data_->primary_key) {
     return Status::InvalidArgument("primary key set for column schema delta", data_->name);
   }
 
-  if (data_->has_rename_to) {
-    col_delta->new_name = boost::optional<string>(std::move(data_->rename_to));
+  if (data_->remove_default && data_->default_val) {
+    return Status::InvalidArgument("new default set but default also removed", data_->name);
   }
-
-  if (data_->has_default) {
-    col_delta->default_value = boost::optional<Slice>(DefaultValueAsSlice());
+
+  if (data_->default_val) {
+    col_delta->default_value = DefaultValueAsSlice();
   }
 
   if (data_->remove_default) {
     col_delta->remove_default = true;
   }
 
-  if (col_delta->remove_default && col_delta->default_value) {
-    return Status::InvalidArgument("new default set but default also removed", data_->name);
-  }
-
-  if (data_->has_encoding) {
-    col_delta->encoding =
-        boost::optional<EncodingType>(ToInternalEncodingType(data_->encoding));
+  if (data_->encoding) {
+    col_delta->encoding = ToInternalEncodingType(data_->encoding.value());
   }
 
-  if (data_->has_compression) {
-    col_delta->compression =
-        boost::optional<CompressionType>(ToInternalCompressionType(data_->compression));
-  }
-
-  if (data_->has_block_size) {
-    col_delta->cfile_block_size = boost::optional<int32_t>(data_->block_size);
+  if (data_->compression) {
+    col_delta->compression = ToInternalCompressionType(data_->compression.value());
   }
 
+  col_delta->new_name = std::move(data_->rename_to);
+  col_delta->cfile_block_size = std::move(data_->block_size);
   col_delta->new_comment = std::move(data_->comment);
   return Status::OK();
 }
 
 Slice KuduColumnSpec::DefaultValueAsSlice() const {
-  if (!data_->has_default) {
+  if (!data_->default_val) {
     return Slice();
   }
-  return data_->default_val->data_->GetSlice();
+  return data_->default_val.value()->data_->GetSlice();
 }
 
 ////////////////////////////////////////////////////////////
@@ -446,7 +424,7 @@ Slice KuduColumnSpec::DefaultValueAsSlice() const {
 
 class KuduSchemaBuilder::Data {
  public:
-  Data() : has_key_col_names(false) {
+  Data() {
   }
 
   ~Data() {
@@ -455,9 +433,7 @@ class KuduSchemaBuilder::Data {
     // headers declaring friend classes with nested classes.
   }
 
-  bool has_key_col_names;
-  vector<string> key_col_names;
-
+  boost::optional<vector<string>> key_col_names;
   vector<KuduColumnSpec*> specs;
 };
 
@@ -475,15 +451,14 @@ KuduSchemaBuilder::~KuduSchemaBuilder() {
   delete data_;
 }
 
-KuduColumnSpec* KuduSchemaBuilder::AddColumn(const std::string& name) {
+KuduColumnSpec* KuduSchemaBuilder::AddColumn(const string& name) {
   auto c = new KuduColumnSpec(name);
   data_->specs.push_back(c);
   return c;
 }
 
 KuduSchemaBuilder* KuduSchemaBuilder::SetPrimaryKey(
-    const std::vector<std::string>& key_col_names) {
-  data_->has_key_col_names = true;
+    const vector<string>& key_col_names) {
   data_->key_col_names = key_col_names;
   return this;
 }
@@ -497,7 +472,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
 
   int num_key_cols;
 
-  if (!data_->has_key_col_names) {
+  if (!data_->key_col_names) {
     // If they didn't explicitly pass the column names for key,
     // then they should have set it on exactly one column.
     int single_key_col_idx = -1;
@@ -541,7 +516,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
 
     // Convert the key column names to a set of indexes.
     vector<int> key_col_indexes;
-    for (const string& key_col_name : data_->key_col_names) {
+    for (const string& key_col_name : data_->key_col_names.value()) {
       int idx;
       if (!FindCopy(name_to_idx_map, key_col_name, &idx)) {
         return Status::InvalidArgument("primary key column not defined", key_col_name);
@@ -555,7 +530,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
     for (int i = 0; i < key_col_indexes.size(); i++) {
       if (key_col_indexes[i] != i) {
         return Status::InvalidArgument("primary key columns must be listed first in the schema",
-                                       data_->key_col_names[i]);
+                                       data_->key_col_names.value()[i]);
       }
     }
 
@@ -575,7 +550,7 @@ Status KuduSchemaBuilder::Build(KuduSchema* schema) {
 // KuduColumnSchema
 ////////////////////////////////////////////////////////////
 
-std::string KuduColumnSchema::DataTypeToString(DataType type) {
+string KuduColumnSchema::DataTypeToString(DataType type) {
   switch (type) {
     case INT8:
       return "INT8";
@@ -603,7 +578,7 @@ std::string KuduColumnSchema::DataTypeToString(DataType type) {
   LOG(FATAL) << "Unhandled type " << type;
 }
 
-KuduColumnSchema::KuduColumnSchema(const std::string &name,
+KuduColumnSchema::KuduColumnSchema(const string &name,
                                    DataType type,
                                    bool is_nullable,
                                    const void* default_value,
@@ -658,7 +633,7 @@ bool KuduColumnSchema::Equals(const KuduColumnSchema& other) const
{
     (col_ != nullptr && col_->Equals(*other.col_, ColumnSchema::COMPARE_ALL));
 }
 
-const std::string& KuduColumnSchema::name() const {
+const string& KuduColumnSchema::name() const {
   return DCHECK_NOTNULL(col_)->name();
 }
 
diff --git a/src/kudu/client/table_alterer-internal.cc b/src/kudu/client/table_alterer-internal.cc
index 4172ec3..56bfcd4 100644
--- a/src/kudu/client/table_alterer-internal.cc
+++ b/src/kudu/client/table_alterer-internal.cc
@@ -95,19 +95,18 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
       }
       case AlterTableRequestPB::ALTER_COLUMN:
       {
-        if (s.spec->data_->has_type ||
-            s.spec->data_->has_nullable ||
+        if (s.spec->data_->type ||
+            s.spec->data_->nullable ||
             s.spec->data_->primary_key) {
           return Status::NotSupported("unsupported alter operation",
                                       s.spec->data_->name);
         }
-        if (!s.spec->data_->has_rename_to &&
-            !s.spec->data_->has_default &&
+        if (!s.spec->data_->rename_to &&
             !s.spec->data_->default_val &&
             !s.spec->data_->remove_default &&
-            !s.spec->data_->has_encoding &&
-            !s.spec->data_->has_compression &&
-            !s.spec->data_->has_block_size &&
+            !s.spec->data_->encoding &&
+            !s.spec->data_->compression &&
+            !s.spec->data_->block_size &&
             !s.spec->data_->comment) {
           return Status::InvalidArgument("no alter operation specified",
                                          s.spec->data_->name);
@@ -115,16 +114,15 @@ Status KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
         // If the alter is solely a column rename, fall back to using
         // RENAME_COLUMN, for backwards compatibility.
         // TODO(wdb) Change this when compat can be broken.
-        if (!s.spec->data_->has_default &&
-            !s.spec->data_->default_val &&
+        if (!s.spec->data_->default_val &&
             !s.spec->data_->remove_default &&
-            !s.spec->data_->has_encoding &&
-            !s.spec->data_->has_compression &&
-            !s.spec->data_->has_block_size &&
+            !s.spec->data_->encoding &&
+            !s.spec->data_->compression &&
+            !s.spec->data_->block_size &&
             !s.spec->data_->comment) {
           pb_step->set_type(AlterTableRequestPB::RENAME_COLUMN);
           pb_step->mutable_rename_column()->set_old_name(s.spec->data_->name);
-          pb_step->mutable_rename_column()->set_new_name(s.spec->data_->rename_to);
+          pb_step->mutable_rename_column()->set_new_name(s.spec->data_->rename_to.value());
           break;
         }
         ColumnSchemaDelta col_delta(s.spec->data_->name);


Mime
View raw message