xiaokang commented on code in PR #31141:
URL: https://github.com/apache/doris/pull/31141#discussion_r1503484255
##########
be/src/olap/tablet_schema.cpp:
##########
@@ -820,21 +822,21 @@ void TabletSchema::append_column(TabletColumn column,
ColumnType col_type) {
} else if (UNLIKELY(column.name() == VERSION_COL)) {
_version_col_idx = _num_columns;
}
+ _field_id_to_index[column.unique_id()] = _num_columns;
Review Comment:
why change the code from last of this function to here?
##########
be/src/olap/tablet_schema.h:
##########
@@ -279,8 +289,8 @@ class TabletSchema {
Status have_column(const std::string& field_name) const;
const TabletColumn& column_by_uid(int32_t col_unique_id) const;
TabletColumn& mutable_column_by_uid(int32_t col_unique_id);
- const std::vector<TabletColumn>& columns() const;
- std::vector<TabletColumn>& mutable_columns();
+ void relace_column(size_t pos, TabletColumn new_col);
Review Comment:
replace_column
##########
be/src/olap/tablet_schema.cpp:
##########
@@ -910,9 +917,9 @@ void TabletSchema::init_from_pb(const TabletSchemaPB&
schema, bool ignore_extrac
if (column.is_variant_type()) {
++_num_variant_columns;
}
- _field_name_to_index[column.name()] = _num_columns;
- _field_id_to_index[column.unique_id()] = _num_columns;
- _cols.emplace_back(std::move(column));
+ _cols.emplace_back(std::make_shared<TabletColumn>(std::move(column)));
Review Comment:
do you mean using StringRef to reuse name string by change order?
##########
be/src/vec/common/schema_util.cpp:
##########
@@ -689,7 +689,9 @@ void rebuild_schema_and_block(const TabletSchemaSPtr&
original,
vectorized::PathInDataBuilder full_root_path_builder;
auto full_root_path =
full_root_path_builder.append(parent_column.name_lower_case(),
false).build();
-
flush_schema->mutable_columns()[variant_pos].set_path_info(full_root_path);
+ TabletColumn new_col = flush_schema->column(variant_pos);
Review Comment:
why not use mutable_columns() as before?
##########
be/src/olap/in_list_predicate.h:
##########
@@ -36,13 +36,6 @@
#include "vec/common/string_ref.h"
#include "vec/core/types.h"
-template <>
-struct std::equal_to<doris::StringRef> {
Review Comment:
should it defined in string_ref.h?
##########
be/src/olap/tablet_schema.h:
##########
@@ -172,29 +181,30 @@ class TabletColumn {
std::string _default_value;
bool _is_decimal = false;
- int32_t _precision;
- int32_t _frac;
+ int32_t _precision = -1;
Review Comment:
why change default values?
##########
be/src/vec/json/path_in_data.cpp:
##########
@@ -118,12 +120,15 @@ void PathInData::from_protobuf(const
segment_v2::ColumnPathInfo& pb) {
path = pb.path();
has_nested = pb.has_has_nested();
parts.reserve(pb.path_part_infos().size());
+ const char* begin = path.data();
for (const segment_v2::ColumnPathPartInfo& part_info :
pb.path_part_infos()) {
Part part;
part.is_nested = part_info.is_nested();
part.anonymous_array_level = part_info.anonymous_array_level();
- part.key = part_info.key();
+ // use string_view to ref data in path
+ part.key = std::string_view {begin, part_info.key().length()};
Review Comment:
part_info may be released when part.key reference it.
##########
be/src/olap/tablet_schema.cpp:
##########
@@ -806,12 +809,11 @@ void TabletSchema::append_column(TabletColumn column,
ColumnType col_type) {
}
if (column.is_variant_type()) {
++_num_variant_columns;
- if (column.path_info().empty()) {
+ if (!column.has_path_info()) {
const std::string& col_name = column.name_lower_case();
vectorized::PathInData path(col_name);
column.set_path_info(path);
}
- _field_path_to_index[column.path_info()] = _num_columns;
Review Comment:
why delete?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
