This is an automated email from the ASF dual-hosted git repository.
alexey 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 20d1824d5 [client] clear error message on column type attributes
20d1824d5 is described below
commit 20d1824d57a91ff56c6a13ac8a69844dbbe2056d
Author: Alexey Serbin <[email protected]>
AuthorDate: Mon Sep 8 20:52:48 2025 -0700
[client] clear error message on column type attributes
This changelist cleans up error status messages issued by the code in
KuduColumnSpec::ToColumnSchema(), for example:
Before:
precision is not valid on a 2 column: key
After:
precision is not applicable to INT32 column: key
I also took the liberty of updating the code to prepare it for follow-up
updates in the context of KUDU-1261.
This changelist doesn't contain any functional modifications.
Change-Id: Ib616af1409e6b2e5010193a42b61209785b37861
Reviewed-on: http://gerrit.cloudera.org:8080/23398
Reviewed-by: Abhishek Chennaka <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
python/kudu/tests/test_schema.py | 6 +-
src/kudu/client/schema.cc | 168 ++++++++++++++++++++++-----------------
2 files changed, 99 insertions(+), 75 deletions(-)
diff --git a/python/kudu/tests/test_schema.py b/python/kudu/tests/test_schema.py
index cbee07b1e..2f010a982 100644
--- a/python/kudu/tests/test_schema.py
+++ b/python/kudu/tests/test_schema.py
@@ -175,7 +175,7 @@ class TestSchema(CompatUnitTest):
.primary_key()
.nullable(False))
- error_msg = 'no precision provided for decimal column: key'
+ error_msg = 'no precision provided for DECIMAL column: key'
with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
@@ -188,7 +188,7 @@ class TestSchema(CompatUnitTest):
.precision(9)
.scale(2))
- error_msg = 'precision is not valid on a 2 column: key'
+ error_msg = 'precision is not applicable to INT32 column: key'
with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
@@ -252,7 +252,7 @@ class TestSchema(CompatUnitTest):
.nullable(False)
.length(10))
- error_msg = 'no precision provided for decimal column: key'
+ error_msg = 'no precision provided for DECIMAL column: key'
with self.assertRaisesRegex(kudu.KuduInvalidArgument, error_msg):
builder.build()
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index df9630a67..d5831000a 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -387,118 +387,142 @@ 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_->rename_to) {
- return Status::NotSupported("cannot rename a column during CreateTable",
- data_->name);
+ return Status::NotSupported("cannot rename a column during CreateTable",
data_->name);
}
if (data_->remove_default) {
- return Status::NotSupported("cannot remove default during CreateTable",
- data_->name);
+ return Status::NotSupported("cannot remove default during CreateTable",
data_->name);
}
-
if (!data_->type) {
return Status::InvalidArgument("no type provided for column", data_->name);
}
- switch (data_->type.value()) {
- case KuduColumnSchema::DECIMAL:
- if (!data_->precision) {
- return Status::InvalidArgument("no precision provided for decimal
column", data_->name);
- }
- if (data_->precision.value() < kMinDecimalPrecision ||
- data_->precision.value() > kMaxDecimalPrecision) {
+ const KuduColumnSchema::DataType column_type = data_->type.value();
+ const auto validate_decimal = [&] {
+ if (!data_->precision) {
+ return Status::InvalidArgument("no precision provided for DECIMAL
column",
+ data_->name);
+ }
+ if (data_->precision.value() < kMinDecimalPrecision ||
+ data_->precision.value() > kMaxDecimalPrecision) {
+ return Status::InvalidArgument(
+ Substitute("precision must be between $0 and $1",
+ kMinDecimalPrecision, kMaxDecimalPrecision),
+ data_->name);
+ }
+ if (data_->scale) {
+ if (data_->scale.value() < kMinDecimalScale) {
return Status::InvalidArgument(
- strings::Substitute("precision must be between $0 and $1",
- kMinDecimalPrecision,
- kMaxDecimalPrecision), data_->name);
- }
- 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.value() > data_->precision.value()) {
- return Status::InvalidArgument(
- strings::Substitute("scale is greater than the precision value
of",
- data_->precision.value()), data_->name);
- }
+ Substitute("scale is less than the minimum value of $0",
+ kMinDecimalScale),
+ data_->name);
}
- if (data_->length) {
+ if (data_->scale.value() > data_->precision.value()) {
return Status::InvalidArgument(
- strings::Substitute("length is not applicable for column $0",
- data_->type.value()), data_->name);
+ Substitute("scale is greater than the precision value of $0",
+ data_->precision.value()),
+ data_->name);
}
+ }
+ if (data_->length) {
+ return Status::InvalidArgument(
+ Substitute("length is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
+ }
+ return Status::OK();
+ };
+
+ const auto validate_varchar = [&] {
+ if (!data_->length) {
+ return Status::InvalidArgument("no length provided for VARCHAR column",
+ data_->name);
+ }
+ if (data_->length.value() < kMinVarcharLength ||
+ data_->length.value() > kMaxVarcharLength) {
+ return Status::InvalidArgument(Substitute("length must be between $0 and
$1",
+ kMinVarcharLength,
+ kMaxVarcharLength),
+ data_->name);
+ }
+ if (data_->precision) {
+ return Status::InvalidArgument(
+ Substitute("precision is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
+ }
+ if (data_->scale) {
+ return Status::InvalidArgument(
+ Substitute("scale is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
+ }
+ return Status::OK();
+ };
+
+ switch (column_type) {
+ case KuduColumnSchema::DECIMAL:
+ RETURN_NOT_OK(validate_decimal());
break;
case KuduColumnSchema::VARCHAR:
- if (!data_->length) {
- return Status::InvalidArgument("no length provided for VARCHAR
column", data_->name);
- }
- if (data_->length.value() < kMinVarcharLength ||
- data_->length.value() > kMaxVarcharLength) {
- return Status::InvalidArgument(
- strings::Substitute("length must be between $0 and $1",
- kMinVarcharLength,
- kMaxVarcharLength), data_->name);
- }
- if (data_->precision) {
- return Status::InvalidArgument(
- strings::Substitute("precision is not valid on a $0 column",
- data_->type.value()), data_->name);
- }
- if (data_->scale) {
- return Status::InvalidArgument(
- strings::Substitute("scale is not valid on a $0 column",
- data_->type.value()), data_->name);
- }
+ RETURN_NOT_OK(validate_varchar());
break;
default:
if (data_->precision) {
return Status::InvalidArgument(
- strings::Substitute("precision is not valid on a $0 column",
- data_->type.value()), data_->name);
+ Substitute("precision is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
}
if (data_->scale) {
return Status::InvalidArgument(
- strings::Substitute("scale is not valid on a $0 column",
- data_->type.value()), data_->name);
+ Substitute("scale is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
}
if (data_->length) {
return Status::InvalidArgument(
- strings::Substitute("length is not valid on a $0 column",
- data_->type.value()), data_->name);
+ Substitute("length is not applicable to $0 column",
+ KuduColumnSchema::DataTypeToString(column_type)),
+ data_->name);
}
+ break;
}
- int8_t precision = data_->precision ? data_->precision.value() : 0;
- int8_t scale = data_->scale ? data_->scale.value() : kDefaultDecimalScale;
- uint16_t length = data_->length ? data_->length.value() : 0;
+ const int8_t precision = data_->precision ? data_->precision.value() : 0;
+ const int8_t scale = data_->scale ? data_->scale.value() :
kDefaultDecimalScale;
+ const uint16_t length = data_->length ? data_->length.value() : 0;
- KuduColumnTypeAttributes type_attrs(precision, scale, length);
- DataType internal_type = ToInternalDataType(data_->type.value(), type_attrs);
- bool nullable = data_->nullable ? data_->nullable.value() : true;
- bool immutable = data_->immutable ? data_->immutable.value() : false;
+ const KuduColumnTypeAttributes type_attrs(precision, scale, length);
+ const DataType internal_type = ToInternalDataType(data_->type.value(),
type_attrs);
+ const bool nullable = data_->nullable ? data_->nullable.value() : true;
+ const bool immutable = data_->immutable ? data_->immutable.value() : false;
void* default_val = nullptr;
// 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.value()->data_->CheckTypeAndGetPointer(
- data_->name, internal_type, internal_type_attrs, &default_val));
+ data_->name, internal_type, ColumnTypeAttributes(precision, scale),
&default_val));
}
// 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;
+ const KuduColumnStorageAttributes::EncodingType encoding =
+ data_->encoding ? data_->encoding.value()
+ : KuduColumnStorageAttributes::AUTO_ENCODING;
+ const KuduColumnStorageAttributes::CompressionType compression =
+ data_->compression ? data_->compression.value()
+ : KuduColumnStorageAttributes::DEFAULT_COMPRESSION;
// BlockSize: '0' signifies server-side default.
- int32_t block_size = data_->block_size ? data_->block_size.value() : 0;
+ const 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.value(), nullable,
- immutable, data_->auto_incrementing, default_val,
+ *col = KuduColumnSchema(data_->name,
+ column_type,
+ nullable,
+ immutable,
+ data_->auto_incrementing,
+ default_val,
KuduColumnStorageAttributes(encoding, compression,
block_size),
type_attrs,
data_->comment ? data_->comment.value() : "");
