You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "amorynan (via GitHub)" <gi...@apache.org> on 2023/04/13 06:37:58 UTC
[GitHub] [doris] amorynan opened a new pull request, #18627: [Refact]( type system) refact serde for type system and pb serde impl
amorynan opened a new pull request, #18627:
URL: https://github.com/apache/doris/pull/18627
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## 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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1507794502
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166254665
##########
be/src/vec/functions/function_rpc.cpp:
##########
@@ -316,6 +317,7 @@ void RPCFnImpl::convert_col_to_pvalue(const ColumnPtr& column, const DataTypePtr
}
}
+// (Todo.Amory) need to delete here
template <bool nullable>
Review Comment:
这个 nullable 模板参数是没用的
--
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
[GitHub] [doris] amorynan commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "amorynan (via GitHub)" <gi...@apache.org>.
amorynan commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1510753793
run p0
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166168681
##########
be/src/vec/data_types/data_type_struct.h:
##########
@@ -99,6 +100,14 @@ class DataTypeStruct final : public IDataType {
std::string to_string(const IColumn& column, size_t row_num) const override;
void to_string(const IColumn& column, size_t row_num, BufferWritable& ostr) const override;
bool get_have_explicit_names() const { return have_explicit_names; }
+ DataTypeSerDeSPtr get_serde() const override {
+ DataTypeSerDeSPtrs ptrs;
+ ptrs.reserve(elems.size());
Review Comment:
这种代码实际没啥用。。。 这里不是很关键，不会有性能问题的，但是多余写的reserve这种逻辑，后期重构或者改可能会改错
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166250291
##########
be/src/vec/data_types/serde/data_type_bitmap_serde.cpp:
##########
@@ -0,0 +1,40 @@
+// 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.
+
+#include "data_type_bitmap_serde.h"
+
+namespace doris {
+
+namespace vectorized {
+Status DataTypeBitMapSerDe::write_column_to_pb(const IColumn& column, PValues& result, int start,
+ int end) const {
+ result.mutable_bytes_value()->Reserve(end - start);
+ for (size_t row_num = start; row_num < end; ++row_num) {
+ StringRef data = column.get_data_at(row_num);
+ result.add_bytes_value(data.to_string());
+ }
Review Comment:
we should consider the nullable
case TypeIndex::BitMap: {
ptype->set_id(PGenericType::BITMAP);
arg->mutable_bytes_value()->Reserve(row_count);
for (size_t row_num = start; row_num < end; ++row_num) {
if constexpr (nullable) {
if (column->is_null_at(row_num)) {
arg->add_bytes_value(nullptr);
} else {
StringRef data = column->get_data_at(row_num);
arg->add_bytes_value(data.data, data.size);
}
} else {
StringRef data = column->get_data_at(row_num);
arg->add_bytes_value(data.data, data.size);
}
}
break;
}
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1165319961
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -0,0 +1,24 @@
+// 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.
+#include "data_type_serde.h"
+
+namespace doris {
+namespace vectorized {
+DataTypeSerDe::DataTypeSerDe() {}
Review Comment:
warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
```suggestion
DataTypeSerDe::DataTypeSerDe() = default;
```
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -0,0 +1,24 @@
+// 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.
+#include "data_type_serde.h"
+
+namespace doris {
+namespace vectorized {
+DataTypeSerDe::DataTypeSerDe() {}
+DataTypeSerDe::~DataTypeSerDe() {}
Review Comment:
warning: use '= default' to define a trivial destructor [modernize-use-equals-default]
```suggestion
DataTypeSerDe::~DataTypeSerDe() = default;
```
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166253167
##########
be/src/vec/data_types/serde/data_type_hll_serde.cpp:
##########
@@ -0,0 +1,41 @@
+// 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.
+
+#include "data_type_hll_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+Status DataTypeHLLSerDe::write_column_to_pb(const IColumn& column, PValues& result, int start,
+ int end) const {
+ result.mutable_bytes_value()->Reserve(end - start);
+ for (size_t row_num = start; row_num < end; ++row_num) {
+ StringRef data = column.get_data_at(row_num);
+ result.add_bytes_value(data.to_string());
Review Comment:
performance maybe bad.
add_bytes(data.c_str, data.size)?
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1510594427
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1168389256
##########
be/test/vec/data_types/serde/data_type_serde_test.cpp:
##########
@@ -0,0 +1,172 @@
+// 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.
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <string>
+
+#include "vec/columns/column_complex.h"
+#include "vec/columns/column_decimal.h"
+#include "vec/columns/column_nullable.h"
+#include "vec/data_types/data_type_bitmap.h"
+#include "vec/data_types/data_type_decimal.h"
+#include "vec/data_types/data_type_hll.h"
+#include "vec/data_types/data_type_nullable.h"
+#include "vec/data_types/data_type_number.h"
+#include "vec/data_types/data_type_quantilestate.h"
+#include "vec/data_types/data_type_string.h"
+
+namespace doris::vectorized {
+
+void column_to_pb(const DataTypePtr data_type, const IColumn& col, PValues* result) {
+ const DataTypeSerDeSPtr serde = data_type->get_serde();
+ serde->write_column_to_pb(col, *result, 0, col.size());
+}
+
+void pb_to_column(const DataTypePtr data_type, PValues& result, IColumn& col) {
+ auto serde = data_type->get_serde();
+ serde->read_column_from_pb(col, result);
+}
+
+void check_pb_col(const DataTypePtr data_type, const IColumn& col) {
+ PValues pv = PValues();
+ column_to_pb(data_type, col, &pv);
+ std::string s1 = pv.DebugString();
+
+ auto col1 = data_type->create_column();
+ pb_to_column(data_type, pv, *col1);
+ PValues as_pv = PValues();
+ column_to_pb(data_type, *col1, &as_pv);
+
+ std::string s2 = as_pv.DebugString();
+ EXPECT_EQ(s1, s2);
+}
+
+void serialize_and_deserialize_pb_test() {
+ // int
+ {
+ auto vec = vectorized::ColumnVector<Int32>::create();
+ auto& data = vec->get_data();
+ for (int i = 0; i < 1024; ++i) {
+ data.push_back(i);
+ }
+ vectorized::DataTypePtr data_type(std::make_shared<vectorized::DataTypeInt32>());
+ check_pb_col(data_type, *vec.get());
+ }
+ // string
+ {
+ auto strcol = vectorized::ColumnString::create();
+ for (int i = 0; i < 1024; ++i) {
+ std::string is = std::to_string(i);
+ strcol->insert_data(is.c_str(), is.size());
+ }
+ vectorized::DataTypePtr data_type(std::make_shared<vectorized::DataTypeString>());
+ check_pb_col(data_type, *strcol.get());
+ }
+ // decimal
+ {
+ vectorized::DataTypePtr decimal_data_type(doris::vectorized::create_decimal(27, 9, true));
+ auto decimal_column = decimal_data_type->create_column();
+ auto& data = ((vectorized::ColumnDecimal<vectorized::Decimal<vectorized::Int128>>*)
+ decimal_column.get())
+ ->get_data();
+ for (int i = 0; i < 1024; ++i) {
+ __int128_t value = i * pow(10, 9) + i * pow(10, 8);
+ data.push_back(value);
+ }
+ check_pb_col(decimal_data_type, *decimal_column.get());
+ }
+ // bitmap
+ {
+ vectorized::DataTypePtr bitmap_data_type(std::make_shared<vectorized::DataTypeBitMap>());
+ auto bitmap_column = bitmap_data_type->create_column();
+ std::vector<BitmapValue>& container =
+ ((vectorized::ColumnBitmap*)bitmap_column.get())->get_data();
+ for (int i = 0; i < 4; ++i) {
+ BitmapValue bv;
+ for (int j = 0; j <= i; ++j) {
+ bv.add(j);
+ }
+ container.push_back(bv);
+ }
+ check_pb_col(bitmap_data_type, *bitmap_column.get());
+ }
+ // hll
+ {
+ vectorized::DataTypePtr hll_data_type(std::make_shared<vectorized::DataTypeHLL>());
+ auto hll_column = hll_data_type->create_column();
+ std::vector<HyperLogLog>& container =
+ ((vectorized::ColumnHLL*)hll_column.get())->get_data();
+ for (int i = 0; i < 4; ++i) {
+ HyperLogLog hll;
+ hll.update(i);
+ container.push_back(hll);
+ }
+ check_pb_col(hll_data_type, *hll_column.get());
+ }
+ // quantilestate
+ {
+ vectorized::DataTypePtr quantile_data_type(
+ std::make_shared<vectorized::DataTypeQuantileStateDouble>());
+ auto quantile_column = quantile_data_type->create_column();
+ std::vector<QuantileStateDouble>& container =
+ ((vectorized::ColumnQuantileStateDouble*)quantile_column.get())->get_data();
+ const long max_rand = 1000000L;
+ double lower_bound = 0;
+ double upper_bound = 100;
+ srandom(time(NULL));
Review Comment:
warning: use nullptr [modernize-use-nullptr]
```suggestion
srandom(time(nullptr));
```
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166174849
##########
be/src/vec/data_types/serde/data_type_decimal_serde.h:
##########
@@ -0,0 +1,64 @@
+// 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.
+
+#include "data_type_serde.h"
+#include "vec/columns/column_decimal.h"
+
+namespace doris {
+
+namespace vectorized {
+
+template <typename T>
+class DataTypeDecimalSerDe : public DataTypeSerDe {
+ static_assert(IsDecimalNumber<T>);
+
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -0,0 +1,24 @@
+// 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.
+#include "data_type_serde.h"
+
+namespace doris {
+namespace vectorized {
+DataTypeSerDe::DataTypeSerDe() = default;
Review Comment:
warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
```suggestion
DataTypeSerDe::DataTypeSerDe() = default;
```
##########
be/src/vec/data_types/serde/data_type_serde.cpp:
##########
@@ -0,0 +1,24 @@
+// 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.
+#include "data_type_serde.h"
+
+namespace doris {
+namespace vectorized {
+DataTypeSerDe::DataTypeSerDe() = default;
+DataTypeSerDe::~DataTypeSerDe() = default;
Review Comment:
warning: use '= default' to define a trivial destructor [modernize-use-equals-default]
```suggestion
DataTypeSerDe::~DataTypeSerDe() = default;
```
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1509858806
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1512338866
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166216409
##########
be/src/vec/data_types/serde/data_type_bitmap_serde.cpp:
##########
@@ -0,0 +1,40 @@
+// 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.
+
+#include "data_type_bitmap_serde.h"
+
+namespace doris {
+
+namespace vectorized {
+Status DataTypeBitMapSerDe::write_column_to_pb(const IColumn& column, PValues& result, int start,
+ int end) const {
+ result.mutable_bytes_value()->Reserve(end - start);
Review Comment:
ptype->set_id(PGenericType::BITMAP); is lost?
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1512338900
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1511018186
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166167349
##########
be/src/vec/data_types/data_type_nullable.h:
##########
@@ -98,6 +99,10 @@ class DataTypeNullable final : public IDataType {
const DataTypePtr& get_nested_type() const { return nested_data_type; }
bool is_null_literal() const override { return nested_data_type->is_null_literal(); }
+ DataTypeSerDeSPtr get_serde() const override {
+ return std::make_shared<DataTypeNullableSerDe>(nested_data_type->get_serde());
Review Comment:
nullable 可能不对，nullable 可能是有2列，一列是nested，另外一列是表示一列是否为null，比如下面的代码：
void RPCFnImpl::convert_nullable_col_to_pvalue(const ColumnPtr& column,
const DataTypePtr& data_type,
const ColumnUInt8& null_col, PValues* arg, int start,
int end) {
int row_count = end - start;
if (column->has_null(row_count)) {
auto* null_map = arg->mutable_null_map();
null_map->Reserve(row_count);
const auto* col = check_and_get_column<ColumnUInt8>(null_col);
auto& data = col->get_data();
null_map->Add(data.begin() + start, data.begin() + end);
RPCFnImpl::convert_col_to_pvalue<true>(column, data_type, arg, start, end);
} else {
RPCFnImpl::convert_col_to_pvalue<false>(column, data_type, arg, start, end);
}
}
template <bool nullable>
void RPCFnImpl::convert_col_to_pvalue(const ColumnPtr& column, const DataTypePtr& data_type,
PValues* arg, int start, int end) {
--
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
[GitHub] [doris] amorynan commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "amorynan (via GitHub)" <gi...@apache.org>.
amorynan commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1510603946
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
[GitHub] [doris] github-actions[bot] commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1511059203
clang-tidy review says "All clean, LGTM! :+1:"
--
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
[GitHub] [doris] amorynan commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "amorynan (via GitHub)" <gi...@apache.org>.
amorynan commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1511014662
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
[GitHub] [doris] amorynan commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "amorynan (via GitHub)" <gi...@apache.org>.
amorynan commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1511047191
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
[GitHub] [doris] hello-stephen commented on pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #18627:
URL: https://github.com/apache/doris/pull/18627#issuecomment-1510646027
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.17 seconds
stream load tsv: 434 seconds loaded 74807831229 Bytes, about 164 MB/s
stream load json: 21 seconds loaded 2358488459 Bytes, about 107 MB/s
stream load orc: 59 seconds loaded 1101869774 Bytes, about 17 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230417033454_clickbench_pr_129844.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
[GitHub] [doris] yiguolei merged pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #18627:
URL: https://github.com/apache/doris/pull/18627
--
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
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1165077989
##########
be/src/vec/data_types/serde/data_type_array_serde.h:
##########
@@ -0,0 +1,34 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeArraySerDe : public DataTypeSerDe {
+public:
+ DataTypeArraySerDe(const DataTypeSerDeSPtr& _nested_serde) : nested_serde(_nested_serde) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
```
##########
be/src/vec/data_types/serde/data_type_decimal_serde.h:
##########
@@ -0,0 +1,64 @@
+// 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.
+
+#include "data_type_serde.h"
+#include "vec/columns/column_decimal.h"
+
+namespace doris {
+
+namespace vectorized {
+
+template <typename T>
+class DataTypeDecimalSerDe : public DataTypeSerDe {
+ static_assert(IsDecimalNumber<T>);
+
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
```
##########
be/src/vec/data_types/serde/data_type_map_serde.h:
##########
@@ -0,0 +1,36 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeMapSerDe : public DataTypeSerDe {
+public:
+ DataTypeMapSerDe(const DataTypeSerDeSPtr& _key_serde, const DataTypeSerDeSPtr& _value_serde)
+ : key_serde(_key_serde), value_serde(_value_serde) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_map_serde.h:
##########
@@ -0,0 +1,36 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeMapSerDe : public DataTypeSerDe {
+public:
+ DataTypeMapSerDe(const DataTypeSerDeSPtr& _key_serde, const DataTypeSerDeSPtr& _value_serde)
+ : key_serde(_key_serde), value_serde(_value_serde) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
```
##########
be/src/vec/data_types/serde/data_type_array_serde.h:
##########
@@ -0,0 +1,34 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeArraySerDe : public DataTypeSerDe {
+public:
+ DataTypeArraySerDe(const DataTypeSerDeSPtr& _nested_serde) : nested_serde(_nested_serde) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_struct_serde.h:
##########
@@ -0,0 +1,35 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeStructSerDe : public DataTypeSerDe {
+public:
+ DataTypeStructSerDe(const DataTypeSerDeSPtrs& _elemSerDeSPtrs)
+ : elemSerDeSPtrs(_elemSerDeSPtrs) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_decimal_serde.h:
##########
@@ -0,0 +1,64 @@
+// 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.
+
+#include "data_type_serde.h"
+#include "vec/columns/column_decimal.h"
+
+namespace doris {
+
+namespace vectorized {
+
+template <typename T>
+class DataTypeDecimalSerDe : public DataTypeSerDe {
+ static_assert(IsDecimalNumber<T>);
+
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_object_serde.h:
##########
@@ -0,0 +1,29 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeObjectSerDe : public DataTypeSerDe {
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
```
##########
be/src/vec/data_types/serde/data_type_object_serde.h:
##########
@@ -0,0 +1,29 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeObjectSerDe : public DataTypeSerDe {
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
```
##########
be/src/vec/data_types/serde/data_type_struct_serde.h:
##########
@@ -0,0 +1,35 @@
+// 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.
+
+#include "data_type_serde.h"
+namespace doris {
+
+namespace vectorized {
+
+class DataTypeStructSerDe : public DataTypeSerDe {
+public:
+ DataTypeStructSerDe(const DataTypeSerDeSPtrs& _elemSerDeSPtrs)
+ : elemSerDeSPtrs(_elemSerDeSPtrs) {}
+
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
```
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166167349
##########
be/src/vec/data_types/data_type_nullable.h:
##########
@@ -98,6 +99,10 @@ class DataTypeNullable final : public IDataType {
const DataTypePtr& get_nested_type() const { return nested_data_type; }
bool is_null_literal() const override { return nested_data_type->is_null_literal(); }
+ DataTypeSerDeSPtr get_serde() const override {
+ return std::make_shared<DataTypeNullableSerDe>(nested_data_type->get_serde());
Review Comment:
nullable 可能不对，nullable 可能是有2列，一列是nested，另外一列是表示一列是否为null，比如下面的代码：
void RPCFnImpl::convert_nullable_col_to_pvalue(const ColumnPtr& column,
const DataTypePtr& data_type,
const ColumnUInt8& null_col, PValues* arg, int start,
int end) {
int row_count = end - start;
if (column->has_null(row_count)) {
auto* null_map = arg->mutable_null_map();
null_map->Reserve(row_count);
const auto* col = check_and_get_column<ColumnUInt8>(null_col);
auto& data = col->get_data();
null_map->Add(data.begin() + start, data.begin() + end);
RPCFnImpl::convert_col_to_pvalue<true>(column, data_type, arg, start, end);
} else {
RPCFnImpl::convert_col_to_pvalue<false>(column, data_type, arg, start, end);
}
}
template <bool nullable>
void RPCFnImpl::convert_col_to_pvalue(const ColumnPtr& column, const DataTypePtr& data_type,
PValues* arg, int start, int end) {
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166168681
##########
be/src/vec/data_types/data_type_struct.h:
##########
@@ -99,6 +100,14 @@ class DataTypeStruct final : public IDataType {
std::string to_string(const IColumn& column, size_t row_num) const override;
void to_string(const IColumn& column, size_t row_num, BufferWritable& ostr) const override;
bool get_have_explicit_names() const { return have_explicit_names; }
+ DataTypeSerDeSPtr get_serde() const override {
+ DataTypeSerDeSPtrs ptrs;
+ ptrs.reserve(elems.size());
Review Comment:
这种代码实际没啥用。。。 这里不是很关键，不会有性能问题的
--
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
[GitHub] [doris] yiguolei commented on a diff in pull request #18627: [Refact](type system) refact serde for type system and pb serde impl
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei commented on code in PR #18627:
URL: https://github.com/apache/doris/pull/18627#discussion_r1166252568
##########
be/src/vec/data_types/serde/data_type_decimal_serde.h:
##########
@@ -0,0 +1,64 @@
+// 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.
+
+#include "data_type_serde.h"
+#include "vec/columns/column_decimal.h"
+
+namespace doris {
+
+namespace vectorized {
+
+template <typename T>
+class DataTypeDecimalSerDe : public DataTypeSerDe {
+ static_assert(IsDecimalNumber<T>);
+
+public:
+ Status write_column_to_pb(const IColumn& column, PValues& result, int start, int end) const override;
+ Status read_column_from_pb(IColumn& column, const PValues& arg) const override;
+};
+
+template <typename T>
+Status DataTypeDecimalSerDe<T>::write_column_to_pb(const IColumn& column, PValues& result,
+ int start, int end) const {
+ int row_count = end - start;
+ const auto* col = check_and_get_column<ColumnDecimal<T>>(column);
+ if constexpr (std::is_same_v<T, Decimal<Int128>>) {
Review Comment:
forget decimal32,64,128i
--
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