compiletheworld commented on code in PR #10003:
URL: https://github.com/apache/incubator-doris/pull/10003#discussion_r891859191
##########
fe/fe-core/src/main/java/org/apache/doris/alter/RollupJobV2.java:
##########
@@ -362,14 +365,26 @@ protected void runWaitingTxnJob() throws
AlterCancelException {
}
}
+ List<Column> fullSchema = tbl.getBaseSchema(true);
+ DescriptorTable descTable = new DescriptorTable();
+ for (Column column : fullSchema) {
+ TupleDescriptor destTupleDesc =
descTable.createTupleDescriptor();
+ SlotDescriptor destSlotDesc =
descTable.addSlotDescriptor(destTupleDesc);
+ destSlotDesc.setIsMaterialized(true);
+ destSlotDesc.setColumn(column);
+ if (column.isAllowNull()) {
Review Comment:
`destSlotDesc.setIsNullable(column.isAllowNull())` seems to be much more
straightforward?
##########
be/src/olap/schema_change.cpp:
##########
@@ -807,22 +807,33 @@ bool
RowBlockAllocator::is_memory_enough_for_sorting(size_t num_rows, size_t all
RowBlockMerger::RowBlockMerger(TabletSharedPtr tablet) : _tablet(tablet) {}
-RowBlockMerger::~RowBlockMerger() {}
+RowBlockMerger::~RowBlockMerger() = default;
bool RowBlockMerger::merge(const std::vector<RowBlock*>& row_block_arr,
RowsetWriter* rowset_writer,
uint64_t* merged_rows) {
uint64_t tmp_merged_rows = 0;
RowCursor row_cursor;
std::unique_ptr<MemPool> mem_pool(new MemPool("RowBlockMerger"));
std::unique_ptr<ObjectPool> agg_object_pool(new ObjectPool());
+
+ auto merge_error = [&]() -> bool {
+ while (_heap.size() > 0) {
+ MergeElement element = _heap.top();
+ _heap.pop();
+ SAFE_DELETE(element.row_cursor);
+ }
+
Review Comment:
lambdas are intended to be small and precise, blank lines should be
eliminated.
##########
be/src/olap/schema_change.cpp:
##########
@@ -1010,16 +1008,6 @@ Status
SchemaChangeDirectly::process(RowsetReaderSharedPtr rowset_reader,
}
Status res = Status::OK();
- if (rowset_reader->rowset()->empty() ||
rowset_reader->rowset()->num_rows() == 0) {
Review Comment:
For an empty rowset, this change will lead to missing data files of rowset,
is it OK that we skip creating an empty rowset?
##########
be/src/olap/schema_change.cpp:
##########
@@ -1065,48 +1049,25 @@ Status
SchemaChangeDirectly::process(RowsetReaderSharedPtr rowset_reader,
return Status::OLAPInternalError(OLAP_ERR_ALTER_STATUS_ERR);
}
- // rows filtered by zone map against delete handler
- add_filtered_rows(rowset_reader->filtered_rows());
-
- // Check row num changes
- if (config::row_nums_check) {
Review Comment:
Why do we skip this check? Is this redundant or useless?
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java:
##########
@@ -374,20 +379,57 @@ protected void runWaitingTxnJob() throws
AlterCancelException {
tbl.readLock();
try {
+ Map<String, Column> indexColumnMap = Maps.newHashMap();
+ for (Map.Entry<Long, List<Column>> entry :
indexSchemaMap.entrySet()) {
+ for (Column column : entry.getValue()) {
+ indexColumnMap.put(column.getName(), column);
+ }
+ }
+
Preconditions.checkState(tbl.getState() ==
OlapTableState.SCHEMA_CHANGE);
for (long partitionId : partitionIndexMap.rowKeySet()) {
Partition partition = tbl.getPartition(partitionId);
Preconditions.checkNotNull(partition, partitionId);
- // the schema change task will transform the data before
visible version(included).
+ // the schema change task will transform the data before
visible
+ // version(included).
long visibleVersion = partition.getVisibleVersion();
Map<Long, MaterializedIndex> shadowIndexMap =
partitionIndexMap.row(partitionId);
for (Map.Entry<Long, MaterializedIndex> entry :
shadowIndexMap.entrySet()) {
long shadowIdxId = entry.getKey();
MaterializedIndex shadowIdx = entry.getValue();
+ Map<String, Expr> defineExprs = Maps.newHashMap();
+
+ List<Column> fullSchema = tbl.getBaseSchema(true);
+ DescriptorTable descTable = new DescriptorTable();
+ for (Column column : fullSchema) {
+ TupleDescriptor destTupleDesc =
descTable.createTupleDescriptor();
+ SlotDescriptor destSlotDesc =
descTable.addSlotDescriptor(destTupleDesc);
+ destSlotDesc.setIsMaterialized(true);
+ destSlotDesc.setColumn(column);
+ if (column.isAllowNull()) {
Review Comment:
The same problems as
`fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java:375`
##########
be/src/olap/schema_change.cpp:
##########
@@ -1119,17 +1080,6 @@ Status
SchemaChangeWithSorting::process(RowsetReaderSharedPtr rowset_reader,
Status res = Status::OK();
RowsetSharedPtr rowset = rowset_reader->rowset();
- if (rowset->empty() || rowset->num_rows() == 0) {
- res = new_rowset_writer->flush();
Review Comment:
Ditto
--
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]
