You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/11/11 10:48:52 UTC
[GitHub] [incubator-doris] morningman commented on a change in pull request #6856: [refactor] replace boost smart ptr with stl
morningman commented on a change in pull request #6856:
URL: https://github.com/apache/incubator-doris/pull/6856#discussion_r747305115
##########
File path: be/src/exec/odbc_connector.cpp
##########
@@ -228,7 +227,7 @@ Status ODBCConnector::append(const std::string& table_name, RowBatch* batch,
}
void* item = _output_expr_ctxs[j]->get_value(row);
if (item == nullptr) {
- fmt::format_to(_insert_stmt_buffer, "{}", "NULL");
+ fmt::format_to(_insert_stmt_buffer, "{}", "nullptr");
Review comment:
This is not right. Here we need literal "NULL", not `nullptr`
##########
File path: be/src/exec/odbc_connector.cpp
##########
@@ -264,11 +263,11 @@ Status ODBCConnector::append(const std::string& table_name, RowBatch* batch,
case TYPE_STRING: {
const auto* string_val = (const StringValue*)(item);
- if (string_val->ptr == NULL) {
+ if (string_val->ptr == nullptr) {
if (string_val->len == 0) {
fmt::format_to(_insert_stmt_buffer, "{}", "''");
} else {
- fmt::format_to(_insert_stmt_buffer, "{}", "NULL");
+ fmt::format_to(_insert_stmt_buffer, "{}", "nullptr");
Review comment:
Save as above
##########
File path: be/src/exprs/expr_context.h
##########
@@ -81,7 +81,7 @@ class ExprContext {
/// result in result_.
void* get_value(TupleRow* row);
- /// Convenience functions: print value into 'str' or 'stream'. NULL turns into "NULL".
+ /// Convenience functions: print value into 'str' or 'stream'. nullptr turns into "nullptr".
Review comment:
```suggestion
/// Convenience functions: print value into 'str' or 'stream'. nullptr turns into "NULL".
```
##########
File path: be/src/exec/parquet_reader.cpp
##########
@@ -167,7 +167,7 @@ inline Status ParquetReaderWrap::set_field_null(Tuple* tuple, const SlotDescript
if (!slot_desc->is_nullable()) {
std::stringstream str_error;
str_error << "The field name(" << slot_desc->col_name()
- << ") is not allowed null, but Parquet field is NULL.";
+ << ") is not allowed null, but Parquet field is nullptr.";
Review comment:
```suggestion
<< ") is not allowed null, but Parquet field is null.";
```
##########
File path: be/src/olap/utils.h
##########
@@ -317,7 +317,7 @@ bool valid_bool(const std::string& value_str);
std::map<int, const char*>::const_iterator it = \
_##enum_type##_VALUES_TO_NAMES.find(index); \
if (it == _##enum_type##_VALUES_TO_NAMES.end()) { \
- out = "NULL"; \
+ out = "nullptr"; \
Review comment:
```suggestion
out = "NULL"; \
```
##########
File path: be/src/olap/snapshot_manager.cpp
##########
@@ -65,14 +63,11 @@ SnapshotManager* SnapshotManager::instance() {
return _s_instance;
}
-OLAPStatus SnapshotManager::make_snapshot(
- const TSnapshotRequest& request,
- string* snapshot_path,
- bool* allow_incremental_clone) {
-
+OLAPStatus SnapshotManager::make_snapshot(const TSnapshotRequest& request, string* snapshot_path,
+ bool* allow_incremental_clone) {
OLAPStatus res = OLAP_SUCCESS;
if (snapshot_path == nullptr) {
- LOG(WARNING) << "output parameter cannot be NULL";
+ LOG(WARNING) << "output parameter cannot be nullptr";
Review comment:
```suggestion
LOG(WARNING) << "output parameter cannot be null";
```
##########
File path: be/src/runtime/mysql_table_writer.cpp
##########
@@ -122,11 +122,11 @@ Status MysqlTableWriter::insert_row(TupleRow* row) {
case TYPE_STRING: {
const StringValue* string_val = (const StringValue*)(item);
- if (string_val->ptr == NULL) {
+ if (string_val->ptr == nullptr) {
if (string_val->len == 0) {
ss << "\'\'";
} else {
- ss << "NULL";
+ ss << "nullptr";
Review comment:
```suggestion
ss << "NULL";
```
##########
File path: be/src/runtime/buffered_block_mgr2.cc
##########
@@ -812,12 +812,12 @@ Status BufferedBlockMgr2::allocate_scratch_space(int64_t block_size, TmpFileMgr:
vector<std::string> errs;
// Find the next physical file in round-robin order and create a write range for it.
for (int attempt = 0; attempt < _tmp_files.size(); ++attempt) {
- *tmp_file = &_tmp_files[_next_block_index];
+ *tmp_file = _tmp_files[_next_block_index].get();
_next_block_index = (_next_block_index + 1) % _tmp_files.size();
- if ((*tmp_file)->is_blacklisted()) {
+ if (_tmp_files[_next_block_index]->is_blacklisted()) {
continue;
}
- Status status = (*tmp_file)->allocate_space(_max_block_size, file_offset);
+ Status status = _tmp_files[_next_block_index]->allocate_space(_max_block_size, file_offset);
Review comment:
```suggestion
Status status = (*tmp_file)->allocate_space(_max_block_size, file_offset);
```
##########
File path: be/src/udf/udf_debug.h
##########
@@ -39,7 +39,7 @@ std::string debug_string(const T& val) {
template <>
std::string debug_string(const StringVal& val) {
if (val.is_null) {
- return "NULL";
+ return "nullptr";
Review comment:
```suggestion
return "NULL";
```
##########
File path: be/src/olap/row_block2.cpp
##########
@@ -100,7 +100,7 @@ std::string RowBlockRow::debug_string() const {
ColumnId cid = _block->schema()->column_ids()[i];
auto col_schema = _block->schema()->column(cid);
if (col_schema->is_nullable() && is_null(cid)) {
- ss << "NULL";
+ ss << "nullptr";
Review comment:
```suggestion
ss << "NULL";
```
##########
File path: be/src/exec/odbc_scan_node.cpp
##########
@@ -48,15 +48,15 @@ Status OdbcScanNode::prepare(RuntimeState* state) {
return Status::OK();
}
- if (NULL == state) {
- return Status::InternalError("input pointer is NULL.");
+ if (nullptr == state) {
+ return Status::InternalError("input pointer is nullptr.");
Review comment:
```suggestion
return Status::InternalError("input pointer is null.");
```
##########
File path: be/src/exec/odbc_scan_node.cpp
##########
@@ -93,8 +93,8 @@ Status OdbcScanNode::open(RuntimeState* state) {
RETURN_IF_ERROR(ExecNode::open(state));
VLOG_CRITICAL << "OdbcScanNode::Open";
- if (NULL == state) {
- return Status::InternalError("input pointer is NULL.");
+ if (nullptr == state) {
+ return Status::InternalError("input pointer is nullptr.");
Review comment:
```suggestion
return Status::InternalError("input pointer is null.");
```
##########
File path: be/src/runtime/mysql_table_writer.cpp
##########
@@ -86,7 +86,7 @@ Status MysqlTableWriter::insert_row(TupleRow* row) {
}
void* item = _output_expr_ctxs[i]->get_value(row);
if (item == nullptr) {
- ss << "NULL";
+ ss << "nullptr";
Review comment:
```suggestion
ss << "NULL";
```
##########
File path: be/src/runtime/collection_value.h
##########
@@ -35,7 +35,7 @@ class ArrayIterator;
* - INT32
* - CHAR
* - VARCHAR
- * - NULL
+ * - nullptr
Review comment:
```suggestion
* - NULL
```
##########
File path: be/src/olap/delete_handler.cpp
##########
@@ -113,7 +113,7 @@ std::string DeleteConditionHandler::construct_sub_predicates(const TCondition& c
bool DeleteConditionHandler::is_condition_value_valid(const TabletColumn& column,
const std::string& condition_op,
const string& value_str) {
- if ("IS" == condition_op && ("NULL" == value_str || "NOT NULL" == value_str)) {
+ if ("IS" == condition_op && ("nullptr" == value_str || "NOT nullptr" == value_str)) {
Review comment:
```suggestion
if ("IS" == condition_op && ("NULL" == value_str || "NOT NULL" == value_str)) {
```
##########
File path: be/src/exprs/like_predicate.cpp
##########
@@ -179,15 +179,15 @@ void LikePredicate::regexp_like_prepare(FunctionContext* context,
// If both the pattern and the match parameter are constant, we pre-compile the
// regular expression once here. Otherwise, the RE is compiled per row in RegexpLike()
if (context->is_arg_constant(1) && context->is_arg_constant(2)) {
- StringVal* pattern = NULL;
+ StringVal* pattern = nullptr;
pattern = reinterpret_cast<StringVal*>(context->get_constant_arg(1));
if (pattern->is_null) {
return;
}
StringVal* match_parameter = reinterpret_cast<StringVal*>(context->get_constant_arg(2));
std::stringstream error;
if (match_parameter->is_null) {
- error << "NULL match parameter";
+ error << "nullptr match parameter";
Review comment:
```suggestion
error << "match parameter is null";
```
##########
File path: be/src/olap/rowset/segment_v2/column_reader.cpp
##########
@@ -649,9 +649,9 @@ Status DefaultValueColumnIterator::init(const ColumnIteratorOptions& opts) {
_opts = opts;
// be consistent with segment v1
// if _has_default_value, we should create default column iterator for this column, and
- // "NULL" is a special default value which means the default value is null.
+ // "nullptr" is a special default value which means the default value is null.
Review comment:
```suggestion
// "NULL" is a special default value which means the default value is null.
```
##########
File path: be/src/olap/row_cursor.cpp
##########
@@ -313,7 +315,7 @@ std::string RowCursor::to_string() const {
result.append(std::to_string(is_null(cid)));
result.append("&");
if (is_null(cid)) {
- result.append("NULL");
+ result.append("nullptr");
Review comment:
```suggestion
result.append("NULL");
```
##########
File path: be/src/olap/rowset/segment_v2/column_reader.cpp
##########
@@ -649,9 +649,9 @@ Status DefaultValueColumnIterator::init(const ColumnIteratorOptions& opts) {
_opts = opts;
// be consistent with segment v1
// if _has_default_value, we should create default column iterator for this column, and
- // "NULL" is a special default value which means the default value is null.
+ // "nullptr" is a special default value which means the default value is null.
if (_has_default_value) {
- if (_default_value == "NULL") {
+ if (_default_value == "nullptr") {
Review comment:
```suggestion
if (_default_value == "NULL") {
```
##########
File path: be/test/udf/udf_test.cpp
##########
@@ -124,39 +123,39 @@ IntVal num_var_args(FunctionContext*, const BigIntVal& dummy, int n, const IntVa
IntVal validat_udf(FunctionContext* context) {
EXPECT_EQ(context->version(), FunctionContext::V2_0);
EXPECT_FALSE(context->has_error());
- EXPECT_TRUE(context->error_msg() == NULL);
+ EXPECT_TRUE(context->error_msg() == nullptr);
return IntVal::null();
}
IntVal validate_fail(FunctionContext* context) {
EXPECT_FALSE(context->has_error());
- EXPECT_TRUE(context->error_msg() == NULL);
+ EXPECT_TRUE(context->error_msg() == nullptr);
context->set_error("Fail");
EXPECT_TRUE(context->has_error());
EXPECT_TRUE(strcmp(context->error_msg(), "Fail") == 0);
return IntVal::null();
}
IntVal validate_mem(FunctionContext* context) {
- EXPECT_TRUE(context->allocate(0) == NULL);
+ EXPECT_TRUE(context->allocate(0) == nullptr);
uint8_t* buffer = context->allocate(10);
- EXPECT_TRUE(buffer != NULL);
+ EXPECT_TRUE(buffer != nullptr);
memset(buffer, 0, 10);
context->free(buffer);
return IntVal::null();
}
-StringVal time_to_string(FunctionContext* context, const TimestampVal& time) {
- boost::posix_time::ptime t(*(boost::gregorian::date*)&time.date);
- t += boost::posix_time::nanoseconds(time.time_of_day);
- std::stringstream ss;
- ss << boost::posix_time::to_iso_extended_string(t) << " "
- << boost::posix_time::to_simple_string(t.time_of_day());
- std::string s = ss.str();
- StringVal result(context, s.size());
- memcpy(result.ptr, s.data(), result.len);
- return result;
-}
+// StringVal time_to_string(FunctionContext* context, const TimestampVal& time) {
Review comment:
delete it if not use?
##########
File path: be/src/runtime/buffered_block_mgr2.cc
##########
@@ -812,12 +812,12 @@ Status BufferedBlockMgr2::allocate_scratch_space(int64_t block_size, TmpFileMgr:
vector<std::string> errs;
// Find the next physical file in round-robin order and create a write range for it.
for (int attempt = 0; attempt < _tmp_files.size(); ++attempt) {
- *tmp_file = &_tmp_files[_next_block_index];
+ *tmp_file = _tmp_files[_next_block_index].get();
_next_block_index = (_next_block_index + 1) % _tmp_files.size();
- if ((*tmp_file)->is_blacklisted()) {
+ if (_tmp_files[_next_block_index]->is_blacklisted()) {
Review comment:
```suggestion
if ((*tmp_file)->is_blacklisted()) {
```
##########
File path: be/src/olap/snapshot_manager.cpp
##########
@@ -312,7 +307,7 @@ OLAPStatus SnapshotManager::_create_snapshot_files(const TabletSharedPtr& ref_ta
<< ", snapshot_version is " << snapshot_version;
OLAPStatus res = OLAP_SUCCESS;
if (snapshot_path == nullptr) {
- LOG(WARNING) << "output parameter cannot be NULL";
+ LOG(WARNING) << "output parameter cannot be nullptr";
Review comment:
```suggestion
LOG(WARNING) << "output parameter cannot be null";
```
##########
File path: be/src/runtime/bufferpool/reservation_tracker.cc
##########
@@ -397,7 +397,7 @@ std::string ReservationTracker::DebugString() {
//std::lock_guard<SpinLock> l(lock_);
if (!initialized_) return "<ReservationTracker>: uninitialized";
- std::string parent_debug_string = parent_ == nullptr ? "NULL" : parent_->DebugString();
+ std::string parent_debug_string = parent_ == nullptr ? "nullptr" : parent_->DebugString();
Review comment:
```suggestion
std::string parent_debug_string = parent_ == nullptr ? "NULL" : parent_->DebugString();
```
##########
File path: be/src/runtime/raw_value.cpp
##########
@@ -194,14 +194,14 @@ void RawValue::print_value(const void* value, const TypeDescriptor& type, int sc
void RawValue::print_value(const void* value, const TypeDescriptor& type, int scale,
std::string* str) {
- if (value == NULL) {
- *str = "NULL";
+ if (value == nullptr) {
+ *str = "nullptr";
Review comment:
```suggestion
*str = "NULL";
```
##########
File path: be/src/runtime/raw_value.cpp
##########
@@ -228,7 +228,7 @@ void RawValue::print_value(const void* value, const TypeDescriptor& type, int sc
return;
}
case TYPE_NULL: {
- *str = "NULL";
+ *str = "nullptr";
Review comment:
```suggestion
*str = "NULL";
```
##########
File path: be/test/exec/mysql_scanner_test.cpp
##########
@@ -71,7 +71,7 @@ TEST_F(MysqlScannerTest, normal_use) {
if (buf[i]) {
LOG(WARNING) << buf[i];
} else {
- LOG(WARNING) << "NULL";
+ LOG(WARNING) << "nullptr";
Review comment:
```suggestion
LOG(WARNING) << "NULL";
```
##########
File path: be/src/runtime/primitive_type.cpp
##########
@@ -180,7 +180,7 @@ std::string type_to_string(PrimitiveType t) {
return "INVALID";
case TYPE_NULL:
- return "NULL";
+ return "nullptr";
Review comment:
```suggestion
return "NULL";
```
##########
File path: be/src/runtime/raw_value.cpp
##########
@@ -97,8 +97,8 @@ void RawValue::print_value_as_bytes(const void* value, const TypeDescriptor& typ
void RawValue::print_value(const void* value, const TypeDescriptor& type, int scale,
std::stringstream* stream) {
- if (value == NULL) {
- *stream << "NULL";
+ if (value == nullptr) {
+ *stream << "nullptr";
Review comment:
```suggestion
*stream << "NULL";
```
##########
File path: be/test/olap/rowset/segment_v2/segment_test.cpp
##########
@@ -660,7 +660,7 @@ TEST_F(SegmentReaderWriterTest, TestDefaultValueColumn) {
// add a column with null default value
{
std::vector<TabletColumn> read_columns = columns;
- read_columns.push_back(create_int_value(5, OLAP_FIELD_AGGREGATION_SUM, true, "NULL"));
+ read_columns.push_back(create_int_value(5, OLAP_FIELD_AGGREGATION_SUM, true, "nullptr"));
Review comment:
```suggestion
read_columns.push_back(create_int_value(5, OLAP_FIELD_AGGREGATION_SUM, true, "NULL"));
```
##########
File path: be/src/olap/rowset/segment_v2/column_reader.cpp
##########
@@ -649,9 +649,9 @@ Status DefaultValueColumnIterator::init(const ColumnIteratorOptions& opts) {
_opts = opts;
// be consistent with segment v1
// if _has_default_value, we should create default column iterator for this column, and
- // "NULL" is a special default value which means the default value is null.
+ // "nullptr" is a special default value which means the default value is null.
if (_has_default_value) {
- if (_default_value == "NULL") {
+ if (_default_value == "nullptr") {
Review comment:
And how about using a global variable `NULL_STRING` to replace all these `"NULL"`
##########
File path: be/src/udf/udf_debug.h
##########
@@ -28,7 +28,7 @@ namespace doris_udf {
template <typename T>
std::string debug_string(const T& val) {
if (val.is_null) {
- return "NULL";
+ return "nullptr";
Review comment:
```suggestion
return "NULL";
```
##########
File path: be/src/olap/snapshot_manager.cpp
##########
@@ -253,7 +248,7 @@ OLAPStatus SnapshotManager::_calc_snapshot_id_path(const TabletSharedPtr& tablet
string* out_path) {
OLAPStatus res = OLAP_SUCCESS;
if (out_path == nullptr) {
- LOG(WARNING) << "output parameter cannot be NULL";
+ LOG(WARNING) << "output parameter cannot be nullptr";
Review comment:
```suggestion
LOG(WARNING) << "output parameter cannot be null";
```
##########
File path: be/src/util/thread_group.h
##########
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <list>
+#include <shared_mutex>
+#include <thread>
+
+#include "common/status.h"
+
+namespace doris {
+class ThreadGroup {
Review comment:
Add ut for this class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org