Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UT for list struct based executors #5534

Closed
Tracked by #4609
SeaRise opened this issue Aug 4, 2022 · 0 comments · Fixed by #5734
Closed
Tracked by #4609

Add UT for list struct based executors #5534

SeaRise opened this issue Aug 4, 2022 · 0 comments · Fixed by #5734
Labels
component/compute type/enhancement The issue or PR belongs to an enhancement.

Comments

@SeaRise
Copy link
Contributor

SeaRise commented Aug 4, 2022

Enhancement

While tidb currently uses tree struct base executor, older versions of tidb and tispark still use list struct base executors.

The difference is that the list struct base executors does not have an executor_id and the list struct base executors is held by DAGRequest.executors.

/// construct DAGQueryBlock from a list struct based executors, which is the
/// format before supporting join in dag request
DAGQueryBlock::DAGQueryBlock(UInt32 id_, const ::google::protobuf::RepeatedPtrField<tipb::Executor> & executors)
: id(id_)
, root(nullptr)
, qb_column_prefix("__QB_" + std::to_string(id_) + "_")
{
for (int i = executors.size() - 1; i >= 0; i--)
{
switch (executors[i].tp())
{
case tipb::ExecType::TypeTableScan:
GET_METRIC(tiflash_coprocessor_executor_count, type_ts).Increment();
assignOrThrowException(&source, &executors[i], SOURCE_NAME);
/// use index as the prefix for executor name so when we sort by
/// the executor name, it will result in the same order as it is
/// in the dag_request, this is needed when filling execution_summary
/// in DAGDriver
if (executors[i].has_executor_id())
source_name = executors[i].executor_id();
else
source_name = std::to_string(i) + "_tablescan";
break;
case tipb::ExecType::TypeSelection:
GET_METRIC(tiflash_coprocessor_executor_count, type_sel).Increment();
assignOrThrowException(&selection, &executors[i], SEL_NAME);
if (executors[i].has_executor_id())
selection_name = executors[i].executor_id();
else
selection_name = std::to_string(i) + "_selection";
break;
case tipb::ExecType::TypeStreamAgg:
case tipb::ExecType::TypeAggregation:
GET_METRIC(tiflash_coprocessor_executor_count, type_agg).Increment();
assignOrThrowException(&aggregation, &executors[i], AGG_NAME);
if (executors[i].has_executor_id())
aggregation_name = executors[i].executor_id();
else
aggregation_name = std::to_string(i) + "_aggregation";
break;
case tipb::ExecType::TypeTopN:
GET_METRIC(tiflash_coprocessor_executor_count, type_topn).Increment();
assignOrThrowException(&limit_or_topn, &executors[i], TOPN_NAME);
if (executors[i].has_executor_id())
limit_or_topn_name = executors[i].executor_id();
else
limit_or_topn_name = std::to_string(i) + "_limitOrTopN";
break;
case tipb::ExecType::TypeLimit:
GET_METRIC(tiflash_coprocessor_executor_count, type_limit).Increment();
assignOrThrowException(&limit_or_topn, &executors[i], LIMIT_NAME);
if (executors[i].has_executor_id())
limit_or_topn_name = executors[i].executor_id();
else
limit_or_topn_name = std::to_string(i) + "_limitOrTopN";
break;
default:
throw TiFlashException(
"Unsupported executor in DAG request: " + executors[i].DebugString(),
Errors::Coprocessor::Unimplemented);
}
}
}

Currently, ExecutorTestUtils only supports tree struct base executor, and we need to find a way to add UTs for list struct base executor.

related issue: #4609

@SeaRise SeaRise added the type/enhancement The issue or PR belongs to an enhancement. label Aug 4, 2022
@SeaRise SeaRise changed the title Add UT for list base executors Add UT for list struct based executors Aug 4, 2022
ti-chi-bot pushed a commit that referenced this issue Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/compute type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant