You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "BiteTheDDDDt (via GitHub)" <gi...@apache.org> on 2023/10/20 09:32:06 UTC
[PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
BiteTheDDDDt opened a new pull request, #25695:
URL: https://github.com/apache/doris/pull/25695
## Proposed changes
remove datetime_value and move vecdatetime_value to doris name space
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #25695:
URL: https://github.com/apache/doris/pull/25695#issuecomment-1772397901
run buildall
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #25695:
URL: https://github.com/apache/doris/pull/25695#issuecomment-1772503607
TeamCity be ut coverage result:
Function Coverage: 37.04% (8301/22410)
Line Coverage: 29.20% (66669/228327)
Region Coverage: 27.83% (34614/124360)
Branch Coverage: 24.42% (17580/71996)
Coverage Report: http://coverage.selectdb-in.cc/coverage/7743f87e58a832862ec1c9969851a07ec28b8ab8_7743f87e58a832862ec1c9969851a07ec28b8ab8/report/index.html
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt merged PR #25695:
URL: https://github.com/apache/doris/pull/25695
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #25695:
URL: https://github.com/apache/doris/pull/25695#discussion_r1366739778
##########
be/src/exec/schema_scanner/schema_tables_scanner.cpp:
##########
@@ -274,7 +274,7 @@
}
// update_time
{
- vectorized::VecDateTimeValue srcs[table_num];
+ VecDateTimeValue srcs[table_num];
Review Comment:
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
```cpp
VecDateTimeValue srcs[table_num];
^
```
##########
be/src/exec/schema_scanner/schema_tables_scanner.cpp:
##########
@@ -255,7 +255,7 @@ Status SchemaTablesScanner::_fill_block_impl(vectorized::Block* block) {
{ static_cast<void>(fill_dest_column_for_range(block, 13, null_datas)); }
// creation_time
{
- vectorized::VecDateTimeValue srcs[table_num];
+ VecDateTimeValue srcs[table_num];
Review Comment:
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
```cpp
VecDateTimeValue srcs[table_num];
^
```
##########
be/src/runtime/fold_constant_executor.cpp:
##########
@@ -205,25 +204,22 @@ string FoldConstantExecutor::_get_result(void* src, size_t size, const TypeDescr
}
case TYPE_DATE:
case TYPE_DATETIME: {
- auto date_value = reinterpret_cast<vectorized::VecDateTimeValue*>(src);
+ auto date_value = reinterpret_cast<VecDateTimeValue*>(src);
Review Comment:
warning: 'auto date_value' can be declared as 'auto *date_value' [readability-qualified-auto]
```suggestion
auto *date_value = reinterpret_cast<VecDateTimeValue*>(src);
```
##########
be/src/util/arrow/block_convertor.cpp:
##########
@@ -129,26 +129,23 @@ class FromBlockConverter : public arrow::TypeVisitor {
case vectorized::TypeIndex::Date:
case vectorized::TypeIndex::DateTime: {
char buf[64];
- const vectorized::VecDateTimeValue* time_val =
- (const vectorized::VecDateTimeValue*)(data_ref.data);
+ const VecDateTimeValue* time_val = (const VecDateTimeValue*)(data_ref.data);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val = (const VecDateTimeValue*)(data_ref.data);
```
##########
be/src/vec/data_types/serde/data_type_datetimev2_serde.cpp:
##########
@@ -99,8 +99,8 @@ void DataTypeDateTimeV2SerDe::write_column_to_arrow(const IColumn& column, const
auto& string_builder = assert_cast<arrow::StringBuilder&>(*array_builder);
for (size_t i = start; i < end; ++i) {
char buf[64];
- const vectorized::DateV2Value<vectorized::DateTimeV2ValueType>* time_val =
- (const vectorized::DateV2Value<vectorized::DateTimeV2ValueType>*)(&col_data[i]);
+ const DateV2Value<DateTimeV2ValueType>* time_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val =
```
##########
be/src/vec/exprs/vexpr.h:
##########
@@ -302,7 +302,7 @@ Status create_texpr_literal_node(const void* data, TExprNode* node, int precisio
(*node).__set_large_int_literal(large_int_literal);
(*node).__set_type(create_type_desc(PrimitiveType::TYPE_LARGEINT));
} else if constexpr ((T == TYPE_DATE) || (T == TYPE_DATETIME) || (T == TYPE_TIME)) {
- auto origin_value = reinterpret_cast<const vectorized::VecDateTimeValue*>(data);
+ auto origin_value = reinterpret_cast<const VecDateTimeValue*>(data);
Review Comment:
warning: 'auto origin_value' can be declared as 'const auto *origin_value' [readability-qualified-auto]
```suggestion
const auto *origin_value = reinterpret_cast<const VecDateTimeValue*>(data);
```
##########
be/src/exec/schema_scanner/schema_tables_scanner.cpp:
##########
@@ -293,7 +293,7 @@
}
// check_time
{
- vectorized::VecDateTimeValue srcs[table_num];
+ VecDateTimeValue srcs[table_num];
Review Comment:
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
```cpp
VecDateTimeValue srcs[table_num];
^
```
##########
be/src/util/arrow/block_convertor.cpp:
##########
@@ -129,26 +129,23 @@
case vectorized::TypeIndex::Date:
case vectorized::TypeIndex::DateTime: {
char buf[64];
- const vectorized::VecDateTimeValue* time_val =
- (const vectorized::VecDateTimeValue*)(data_ref.data);
+ const VecDateTimeValue* time_val = (const VecDateTimeValue*)(data_ref.data);
int len = time_val->to_buffer(buf);
ARROW_RETURN_NOT_OK(builder.Append(buf, len));
break;
}
case vectorized::TypeIndex::DateV2: {
char buf[64];
- const vectorized::DateV2Value<vectorized::DateV2ValueType>* time_val =
- (const vectorized::DateV2Value<
- vectorized::DateV2ValueType>*)(data_ref.data);
+ const DateV2Value<DateV2ValueType>* time_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val =
```
##########
be/src/olap/in_list_predicate.h:
##########
@@ -142,17 +142,15 @@
} else if constexpr (Type == TYPE_DATE) {
HybridSetBase::IteratorBase* iter = hybrid_set->begin();
while (iter->has_next()) {
- const vectorized::VecDateTimeValue* value =
- (const vectorized::VecDateTimeValue*)(iter->get_value());
+ const VecDateTimeValue* value = (const VecDateTimeValue*)(iter->get_value());
uint64_t date = value->to_olap_date();
_values->insert(&date);
iter->next();
}
} else if constexpr (Type == TYPE_DATETIME) {
HybridSetBase::IteratorBase* iter = hybrid_set->begin();
while (iter->has_next()) {
- const vectorized::VecDateTimeValue* value =
- (const vectorized::VecDateTimeValue*)(iter->get_value());
+ const VecDateTimeValue* value = (const VecDateTimeValue*)(iter->get_value());
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* value = (const VecDateTimeValue*)(iter->get_value());
```
##########
be/src/util/static_asserts.cpp:
##########
@@ -31,7 +31,7 @@ class UnusedClass {
private:
static_assert(sizeof(StringRef) == 16);
static_assert(offsetof(StringRef, size) == 8);
- static_assert(sizeof(doris::vectorized::VecDateTimeValue) == 8);
+ static_assert(sizeof(doris::VecDateTimeValue) == 8);
Review Comment:
warning: 8 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
```cpp
static_assert(sizeof(doris::VecDateTimeValue) == 8);
^
```
##########
be/src/exec/es/es_scroll_parser.cpp:
##########
@@ -188,7 +188,7 @@ Status get_int_value(const rapidjson::Value& col, PrimitiveType type, void* slot
template <typename T, typename RT>
Status get_date_value_int(const rapidjson::Value& col, PrimitiveType type, bool is_date_str,
RT* slot, const cctz::time_zone& time_zone) {
- constexpr bool is_datetime_v1 = std::is_same_v<T, vectorized::VecDateTimeValue>;
+ constexpr bool is_datetime_v1 = std::is_same_v<T, VecDateTimeValue>;
Review Comment:
warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]
```cpp
constexpr bool is_datetime_v1 = std::is_same_v<T, VecDateTimeValue>;
^
```
##########
be/src/util/arrow/block_convertor.cpp:
##########
@@ -129,26 +129,23 @@
case vectorized::TypeIndex::Date:
case vectorized::TypeIndex::DateTime: {
char buf[64];
- const vectorized::VecDateTimeValue* time_val =
- (const vectorized::VecDateTimeValue*)(data_ref.data);
+ const VecDateTimeValue* time_val = (const VecDateTimeValue*)(data_ref.data);
int len = time_val->to_buffer(buf);
ARROW_RETURN_NOT_OK(builder.Append(buf, len));
break;
}
case vectorized::TypeIndex::DateV2: {
char buf[64];
- const vectorized::DateV2Value<vectorized::DateV2ValueType>* time_val =
- (const vectorized::DateV2Value<
- vectorized::DateV2ValueType>*)(data_ref.data);
+ const DateV2Value<DateV2ValueType>* time_val =
+ (const DateV2Value<DateV2ValueType>*)(data_ref.data);
int len = time_val->to_buffer(buf);
ARROW_RETURN_NOT_OK(builder.Append(buf, len));
break;
}
case vectorized::TypeIndex::DateTimeV2: {
char buf[64];
- const vectorized::DateV2Value<vectorized::DateTimeV2ValueType>* time_val =
- (const vectorized::DateV2Value<
- vectorized::DateTimeV2ValueType>*)(data_ref.data);
+ const DateV2Value<DateTimeV2ValueType>* time_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val =
```
##########
be/src/runtime/raw_value.h:
##########
@@ -76,7 +76,7 @@ inline uint32_t RawValue::zlib_crc32(const void* v, size_t len, const PrimitiveT
return HashUtil::zlib_crc_hash(v, 8, seed);
case TYPE_DATE:
case TYPE_DATETIME: {
- auto* date_val = (const vectorized::VecDateTimeValue*)v;
+ auto* date_val = (const VecDateTimeValue*)v;
Review Comment:
warning: 'auto *date_val' can be declared as 'const auto *date_val' [readability-qualified-auto]
```suggestion
const auto* date_val = (const VecDateTimeValue*)v;
```
##########
be/src/util/bitmap_intersect.h:
##########
@@ -54,8 +54,7 @@ class Helper {
};
template <>
-char* Helper::write_to<vectorized::VecDateTimeValue>(const vectorized::VecDateTimeValue& v,
- char* dest) {
+char* Helper::write_to<VecDateTimeValue>(const VecDateTimeValue& v, char* dest) {
Review Comment:
warning: method 'write_to' can be made static [readability-convert-member-functions-to-static]
be/src/util/bitmap_intersect.h:55:
```diff
- template <>
+ static template <>
```
##########
be/src/olap/in_list_predicate.h:
##########
@@ -142,17 +142,15 @@ class InListPredicateBase : public ColumnPredicate {
} else if constexpr (Type == TYPE_DATE) {
HybridSetBase::IteratorBase* iter = hybrid_set->begin();
while (iter->has_next()) {
- const vectorized::VecDateTimeValue* value =
- (const vectorized::VecDateTimeValue*)(iter->get_value());
+ const VecDateTimeValue* value = (const VecDateTimeValue*)(iter->get_value());
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* value = (const VecDateTimeValue*)(iter->get_value());
```
##########
be/src/vec/data_types/serde/data_type_date64_serde.cpp:
##########
@@ -173,8 +171,7 @@ void DataTypeDate64SerDe::write_column_to_arrow(const IColumn& column, const Nul
auto& string_builder = assert_cast<arrow::StringBuilder&>(*array_builder);
for (size_t i = start; i < end; ++i) {
char buf[64];
- const vectorized::VecDateTimeValue* time_val =
- (const vectorized::VecDateTimeValue*)(&col_data[i]);
+ const VecDateTimeValue* time_val = (const VecDateTimeValue*)(&col_data[i]);
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val = (const VecDateTimeValue*)(&col_data[i]);
```
##########
be/src/vec/exprs/vexpr.h:
##########
@@ -327,9 +326,7 @@
(*node).__set_node_type(TExprNodeType::DATE_LITERAL);
(*node).__set_type(create_type_desc(PrimitiveType::TYPE_DATEV2));
} else if constexpr (T == TYPE_DATETIMEV2) {
- auto origin_value =
- reinterpret_cast<const vectorized::DateV2Value<vectorized::DateTimeV2ValueType>*>(
- data);
+ auto origin_value = reinterpret_cast<const DateV2Value<DateTimeV2ValueType>*>(data);
Review Comment:
warning: 'auto origin_value' can be declared as 'const auto *origin_value' [readability-qualified-auto]
```suggestion
const auto *origin_value = reinterpret_cast<const DateV2Value<DateTimeV2ValueType>*>(data);
```
##########
be/src/vec/data_types/serde/data_type_datev2_serde.cpp:
##########
@@ -88,8 +88,8 @@ void DataTypeDateV2SerDe::write_column_to_arrow(const IColumn& column, const Nul
auto& string_builder = assert_cast<arrow::StringBuilder&>(*array_builder);
for (size_t i = start; i < end; ++i) {
char buf[64];
- const vectorized::DateV2Value<vectorized::DateV2ValueType>* time_val =
- (const vectorized::DateV2Value<vectorized::DateV2ValueType>*)(&col_data[i]);
+ const DateV2Value<DateV2ValueType>* time_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
const auto* time_val =
```
##########
be/src/vec/exprs/vexpr.h:
##########
@@ -317,8 +317,7 @@
(*node).__set_type(create_type_desc(PrimitiveType::TYPE_TIME));
}
} else if constexpr (T == TYPE_DATEV2) {
- auto origin_value =
- reinterpret_cast<const vectorized::DateV2Value<vectorized::DateV2ValueType>*>(data);
+ auto origin_value = reinterpret_cast<const DateV2Value<DateV2ValueType>*>(data);
Review Comment:
warning: 'auto origin_value' can be declared as 'const auto *origin_value' [readability-qualified-auto]
```suggestion
const auto *origin_value = reinterpret_cast<const DateV2Value<DateV2ValueType>*>(data);
```
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #25695:
URL: https://github.com/apache/doris/pull/25695#issuecomment-1772545732
(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 45.9 seconds
stream load tsv: 554 seconds loaded 74807831229 Bytes, about 128 MB/s
stream load json: 20 seconds loaded 2358488459 Bytes, about 112 MB/s
stream load orc: 64 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 31 seconds loaded 861443392 Bytes, about 26 MB/s
insert into select: 28.9 seconds inserted 10000000 Rows, about 346K ops/s
storage size: 17161974140 Bytes
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25695:
URL: https://github.com/apache/doris/pull/25695#issuecomment-1772807587
PR approved by at least one committer and no changes requested.
--
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
Re: [PR] [Chore](type) remove datetime_value and move vecdatetime_value to doris name space [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25695:
URL: https://github.com/apache/doris/pull/25695#issuecomment-1772807641
PR approved by anyone and no changes requested.
--
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