This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch branch-1.18.x in repository https://gitbox.apache.org/repos/asf/kudu.git
commit a4394a4b6c7dcb7d0030a2b7fd0190bd0090b766 Author: Alexey Serbin <[email protected]> AuthorDate: Wed Feb 12 14:53:22 2025 -0800 [common] small clean-up on PartialRow It's possible to use 'if constexpr' to enable compile-time optimisations in template code since switching to C++17. This patch updates the implementations of KuduPartialRow::{Set,SetSliceCopy}() methods to use compile-time evaluation of 'type' and 'physical_type'. I also added static_assert() and PREDICT_FALSE() in the modified code where it's appropriate, and did a few other style-related updates. Change-Id: I1b164eb7ec7f010e9228175a026aa63726a909c6 Reviewed-on: http://gerrit.cloudera.org:8080/22480 Tested-by: Alexey Serbin <[email protected]> Reviewed-by: Yifan Zhang <[email protected]> (cherry picked from commit cc5f132032abf94dec22f666b637678388448afc) Reviewed-on: http://gerrit.cloudera.org:8080/22483 Reviewed-by: Abhishek Chennaka <[email protected]> --- src/kudu/common/partial_row.cc | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/kudu/common/partial_row.cc b/src/kudu/common/partial_row.cc index 3f50d165c..e7b7ebb9b 100644 --- a/src/kudu/common/partial_row.cc +++ b/src/kudu/common/partial_row.cc @@ -142,7 +142,9 @@ Status KuduPartialRow::Set(int col_idx, ContiguousRow row(schema_, row_data_); // If we're replacing an existing STRING/BINARY value, deallocate the old value. - if (T::physical_type == BINARY) DeallocateStringIfSet(col_idx, col); + if constexpr (T::physical_type == BINARY) { + DeallocateStringIfSet(col_idx, col); + } // Mark the column as set. BitmapSet(isset_bitmap_, col_idx); @@ -422,7 +424,7 @@ Status KuduPartialRow::SetVarcharNoCopyUnsafe(const Slice& col_name, const Slice Status KuduPartialRow::SetVarcharNoCopyUnsafe(int col_idx, const Slice& val) { const auto& col = schema_->column(col_idx); - if (val.size() > col.type_attributes().length * 4) { + if (PREDICT_FALSE(val.size() > col.type_attributes().length * 4)) { return Status::InvalidArgument( Substitute("Value too long, limit is $0 characters", col.type_attributes().length)); @@ -439,21 +441,21 @@ Status KuduPartialRow::SetSliceCopy(const Slice& col_name, const Slice& val) { template<typename T> Status KuduPartialRow::SetSliceCopy(int col_idx, const Slice& val) { + static_assert(T::type == STRING || T::type == BINARY || T::type == VARCHAR, + "incompatible type for SetSliceCopy"); const auto& col = schema_->column(col_idx); Slice relocated_val; - switch (T::type) { - case VARCHAR: - relocated_val = UTF8Truncate(val, col.type_attributes().length); - break; - case STRING: - case BINARY: - auto relocated = new uint8_t[val.size()]; + if constexpr (T::type == VARCHAR) { + relocated_val = UTF8Truncate(val, col.type_attributes().length); + } else { + if constexpr (T::type == STRING || T::type == BINARY) { + auto* relocated = new uint8_t[val.size()]; memcpy(relocated, val.data(), val.size()); relocated_val = Slice(relocated, val.size()); - break; + } } - Status s = Set<T>(col_idx, relocated_val, true); - if (!s.ok()) { + Status s = Set<T>(col_idx, relocated_val, /*owned=*/true); + if (PREDICT_FALSE(!s.ok())) { delete [] relocated_val.data(); } return s; @@ -471,7 +473,9 @@ Status KuduPartialRow::SetNull(int col_idx) { return Status::InvalidArgument("column not nullable", col.ToString()); } - if (col.type_info()->physical_type() == BINARY) DeallocateStringIfSet(col_idx, col); + if (col.type_info()->physical_type() == BINARY) { + DeallocateStringIfSet(col_idx, col); + } ContiguousRow row(schema_, row_data_); row.set_null(col_idx, true); @@ -489,7 +493,9 @@ Status KuduPartialRow::Unset(const Slice& col_name) { Status KuduPartialRow::Unset(int col_idx) { const ColumnSchema& col = schema_->column(col_idx); - if (col.type_info()->physical_type() == BINARY) DeallocateStringIfSet(col_idx, col); + if (col.type_info()->physical_type() == BINARY) { + DeallocateStringIfSet(col_idx, col); + } BitmapClear(isset_bitmap_, col_idx); return Status::OK(); }
