dataroaring commented on code in PR #24788:
URL: https://github.com/apache/doris/pull/24788#discussion_r1395120045
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -463,6 +470,26 @@ Status Segment::lookup_row_key(const Slice& key, bool
with_seq_col, RowLocation*
"key with higher sequence id exists");
}
}
+ if (!has_seq_col && has_rowid) {
Review Comment:
else if (has_row_id)
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -2339,9 +2340,22 @@ private boolean checkPushSort(SortNode sortNode,
OlapTable olapTable) {
if (sortExprs.size() > olapTable.getDataSortInfo().getColNum()) {
return false;
}
+ List<Column> sortKeyColumns = olapTable.getFullSchema();
+ if (olapTable.getEnableUniqueKeyMergeOnWrite()) {
+ Map<Integer, Column> clusterKeyMap = new TreeMap<>();
+ for (Column column : olapTable.getFullSchema()) {
+ if (column.getClusterKeyId() != -1) {
+ clusterKeyMap.put(column.getClusterKeyId(), column);
+ }
+ }
+ if (!clusterKeyMap.isEmpty()) {
+ sortKeyColumns.clear();
+ sortKeyColumns.addAll(clusterKeyMap.values());
+ }
+ }
for (int i = 0; i < sortExprs.size(); i++) {
// table key.
- Column tableKey = olapTable.getFullSchema().get(i);
+ Column tableKey = sortKeyColumns.get(i);
Review Comment:
sortColumn may be a better name than table Key.
##########
be/src/olap/rowset/segment_v2/segment_iterator.cpp:
##########
@@ -1261,6 +1262,12 @@ Status
SegmentIterator::_lookup_ordinal_from_pk_index(const RowCursor& key, bool
// The sequence column needs to be removed from primary key index when
comparing key
bool has_seq_col = _segment->_tablet_schema->has_sequence_col();
+ bool has_rowid = !_segment->_tablet_schema->cluster_key_idxes().empty();
+ size_t rowid_length = 0;
+ if (has_rowid) {
+ rowid_length = sizeof(uint32_t) + 1;
Review Comment:
rowid_length is used in several place as a constant, so we should define a
constant with comment.
##########
be/src/olap/merger.cpp:
##########
@@ -341,6 +356,22 @@ Status Merger::vertical_merge_rowsets(TabletSharedPtr
tablet, ReaderType reader_
std::vector<std::vector<uint32_t>> column_groups;
vertical_split_columns(tablet_schema, &column_groups);
+ std::vector<uint32_t> key_group_cluster_key_idxes;
+ if (column_groups.size() > 0) {
+ if (!tablet_schema->cluster_key_idxes().empty()) {
+ auto& key_column_group = column_groups[0];
+ for (const auto& index_in_tablet_schema :
tablet_schema->cluster_key_idxes()) {
+ for (auto j = 0; j < key_column_group.size(); ++j) {
+ auto cid = key_column_group[j];
+ if (cid == index_in_tablet_schema) {
+ key_group_cluster_key_idxes.emplace_back(j);
+ break;
+ }
+ }
+ }
Review Comment:
we can put above code to a method, too much ident here.
##########
be/src/olap/merger.cpp:
##########
@@ -164,16 +167,23 @@ void Merger::vertical_split_columns(TabletSchemaSPtr
tablet_schema,
if (delete_sign_idx != -1) {
key_columns.emplace_back(delete_sign_idx);
}
+ if (!tablet_schema->cluster_key_idxes().empty()) {
+ for (const auto& cid : tablet_schema->cluster_key_idxes()) {
+ if (cid >= num_key_cols) {
+ key_columns.emplace_back(cid);
Review Comment:
we can rename key_columns to sort_columns.
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -849,6 +849,8 @@ nonterminal DistributionDesc opt_distribution;
nonterminal Integer opt_distribution_number;
nonterminal Long opt_field_length;
nonterminal KeysDesc opt_keys;
+nonterminal KeysDesc opt_mv_keys;
Review Comment:
useless?
--
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-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
