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