This is an automated email from the ASF dual-hosted git repository.
yangzy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-gluten.git
The following commit(s) were added to refs/heads/main by this push:
new fd8ff2dc4 [VL] Refine log plan/split json into one line
fd8ff2dc4 is described below
commit fd8ff2dc438456df8b4b369512c6cc4a41dc71e7
Author: Yang Zhang <yangchuan...@alibaba-inc.com>
AuthorDate: Wed Mar 13 11:08:17 2024 +0800
[VL] Refine log plan/split json into one line
Refine log plan/split json into one line, it helps filter json output
quickly when multi task running.
Also, fix some warnings reported by IDE.
---
cpp/velox/compute/VeloxPlanConverter.cc | 2 +-
cpp/velox/compute/VeloxRuntime.cc | 11 +++++------
cpp/velox/compute/WholeStageResultIterator.cc | 2 +-
cpp/velox/substrait/SubstraitParser.cc | 4 ++--
cpp/velox/substrait/SubstraitToVeloxPlan.cc | 18 +++++++++---------
cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc | 12 ++++++------
6 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/cpp/velox/compute/VeloxPlanConverter.cc
b/cpp/velox/compute/VeloxPlanConverter.cc
index cffbd986e..47738cff9 100644
--- a/cpp/velox/compute/VeloxPlanConverter.cc
+++ b/cpp/velox/compute/VeloxPlanConverter.cc
@@ -103,7 +103,7 @@ void parseLocalFileNodes(
const auto& localFile = localFiles[i];
const auto& fileList = localFile.items();
- splitInfos.push_back(std::move(parseScanSplitInfo(fileList)));
+ splitInfos.push_back(parseScanSplitInfo(fileList));
}
planConverter->setSplitInfos(std::move(splitInfos));
diff --git a/cpp/velox/compute/VeloxRuntime.cc
b/cpp/velox/compute/VeloxRuntime.cc
index 654d374a3..f8f2a527c 100644
--- a/cpp/velox/compute/VeloxRuntime.cc
+++ b/cpp/velox/compute/VeloxRuntime.cc
@@ -53,9 +53,8 @@ void VeloxRuntime::parsePlan(
taskInfo_ = taskInfo;
if (debugModeEnabled_) {
try {
- auto jsonPlan = substraitFromPbToJson("Plan", data, size, dumpFile);
- LOG(INFO) << std::string(50, '#') << " received substrait::Plan:";
- LOG(INFO) << taskInfo_ << std::endl << jsonPlan;
+ auto planJson = substraitFromPbToJson("Plan", data, size, dumpFile);
+ LOG(INFO) << std::string(50, '#') << " received substrait::Plan: " <<
taskInfo_ << std::endl << planJson;
} catch (const std::exception& e) {
LOG(WARNING) << "Error converting Substrait plan to JSON: " << e.what();
}
@@ -67,9 +66,9 @@ void VeloxRuntime::parsePlan(
void VeloxRuntime::parseSplitInfo(const uint8_t* data, int32_t size,
std::optional<std::string> dumpFile) {
if (debugModeEnabled_) {
try {
- auto jsonPlan = substraitFromPbToJson("ReadRel.LocalFiles", data, size,
dumpFile);
- LOG(INFO) << std::string(50, '#') << " received
substrait::ReadRel.LocalFiles:";
- LOG(INFO) << std::endl << jsonPlan;
+ auto splitJson = substraitFromPbToJson("ReadRel.LocalFiles", data, size,
dumpFile);
+ LOG(INFO) << std::string(50, '#') << " received
substrait::ReadRel.LocalFiles: " << taskInfo_ << std::endl
+ << splitJson;
} catch (const std::exception& e) {
LOG(WARNING) << "Error converting Substrait plan to JSON: " << e.what();
}
diff --git a/cpp/velox/compute/WholeStageResultIterator.cc
b/cpp/velox/compute/WholeStageResultIterator.cc
index 1b217ebeb..b10c643c8 100644
--- a/cpp/velox/compute/WholeStageResultIterator.cc
+++ b/cpp/velox/compute/WholeStageResultIterator.cc
@@ -522,7 +522,7 @@ std::shared_ptr<velox::Config>
WholeStageResultIterator::createConnectorConfig()
std::unordered_map<std::string, std::string> configs = {};
// The semantics of reading as lower case is opposite with case-sensitive.
configs[velox::connector::hive::HiveConfig::kFileColumnNamesReadAsLowerCaseSession]
=
- veloxCfg_->get<bool>(kCaseSensitive, false) == false ? "true" : "false";
+ !veloxCfg_->get<bool>(kCaseSensitive, false) ? "true" : "false";
configs[velox::connector::hive::HiveConfig::kPartitionPathAsLowerCaseSession] =
"false";
configs[velox::connector::hive::HiveConfig::kArrowBridgeTimestampUnit] = "6";
configs[velox::connector::hive::HiveConfig::kMaxPartitionsPerWritersSession]
=
diff --git a/cpp/velox/substrait/SubstraitParser.cc
b/cpp/velox/substrait/SubstraitParser.cc
index 479e41876..36fe84558 100644
--- a/cpp/velox/substrait/SubstraitParser.cc
+++ b/cpp/velox/substrait/SubstraitParser.cc
@@ -151,8 +151,8 @@ std::vector<std::string> SubstraitParser::makeNames(const
std::string& prefix, i
return names;
}
-std::string SubstraitParser::makeNodeName(int node_id, int col_idx) {
- return fmt::format("n{}_{}", node_id, col_idx);
+std::string SubstraitParser::makeNodeName(int nodeId, int colIdx) {
+ return fmt::format("n{}_{}", nodeId, colIdx);
}
int SubstraitParser::getIdxFromNodeName(const std::string& nodeName) {
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlan.cc
b/cpp/velox/substrait/SubstraitToVeloxPlan.cc
index 100dcb9eb..d9ebe2902 100644
--- a/cpp/velox/substrait/SubstraitToVeloxPlan.cc
+++ b/cpp/velox/substrait/SubstraitToVeloxPlan.cc
@@ -659,7 +659,7 @@ core::PlanNodePtr
SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
makeLocationHandle(writePath),
dwio::common::FileFormat::PARQUET, // Currently only support
parquet format.
compressionCodec)),
- (partitionedKey.size() > 0) ? true : false,
+ (!partitionedKey.empty()),
exec::TableWriteTraits::outputType(nullptr),
connector::CommitStrategy::kNoCommit,
childNode);
@@ -2005,13 +2005,13 @@ void
SubstraitToVeloxPlanConverter::constructSubfieldFilters(
// Not equal.
if (filterInfo.notValue_) {
filters[common::Subfield(inputName)] =
-
std::move(std::make_unique<common::BoolValue>(!filterInfo.notValue_.value().value<bool>(),
nullAllowed));
+
std::make_unique<common::BoolValue>(!filterInfo.notValue_.value().value<bool>(),
nullAllowed);
} else if (rangeSize == 0) {
// IsNull/IsNotNull.
if (!nullAllowed) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNotNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNotNull>();
} else if (isNull) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNull>();
} else {
VELOX_NYI("Only IsNotNull and IsNull are supported in
constructSubfieldFilters when no other filter ranges.");
}
@@ -2020,15 +2020,15 @@ void
SubstraitToVeloxPlanConverter::constructSubfieldFilters(
// Equal.
auto value = filterInfo.lowerBounds_[0].value().value<bool>();
VELOX_CHECK(value == filterInfo.upperBounds_[0].value().value<bool>(),
"invalid state of bool equal");
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::BoolValue>(value, nullAllowed));
+ filters[common::Subfield(inputName)] =
std::make_unique<common::BoolValue>(value, nullAllowed);
}
} else if constexpr (KIND == facebook::velox::TypeKind::ARRAY || KIND ==
facebook::velox::TypeKind::MAP) {
// Only IsNotNull and IsNull are supported for array and map types.
if (rangeSize == 0) {
if (!nullAllowed) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNotNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNotNull>();
} else if (isNull) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNull>();
} else {
VELOX_NYI(
"Only IsNotNull and IsNull are supported in
constructSubfieldFilters for input type '{}'.",
@@ -2082,9 +2082,9 @@ void
SubstraitToVeloxPlanConverter::constructSubfieldFilters(
// Handle null filtering.
if (rangeSize == 0) {
if (!nullAllowed) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNotNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNotNull>();
} else if (isNull) {
- filters[common::Subfield(inputName)] =
std::move(std::make_unique<common::IsNull>());
+ filters[common::Subfield(inputName)] =
std::make_unique<common::IsNull>();
} else {
VELOX_NYI("Only IsNotNull and IsNull are supported in
constructSubfieldFilters when no other filter ranges.");
}
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
index da7fb3cdc..b15b18872 100644
--- a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
+++ b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
@@ -648,8 +648,8 @@ bool SubstraitToVeloxPlanValidator::validate(const
::substrait::WindowRel& windo
try {
for (const auto& expr : groupByExprs) {
auto expression = exprConverter_->toVeloxExpr(expr, rowType);
- auto expr_field = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
- if (expr_field == nullptr) {
+ auto exprField = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
+ if (exprField == nullptr) {
LOG_VALIDATION_MSG("Only field is supported for partition key in
Window Operator!");
return false;
} else {
@@ -681,8 +681,8 @@ bool SubstraitToVeloxPlanValidator::validate(const
::substrait::WindowRel& windo
if (sort.has_expr()) {
try {
auto expression = exprConverter_->toVeloxExpr(sort.expr(), rowType);
- auto expr_field = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
- if (!expr_field) {
+ auto exprField = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
+ if (!exprField) {
LOG_VALIDATION_MSG("in windowRel, the sorting key in Sort Operator
only support field.");
return false;
}
@@ -739,8 +739,8 @@ bool SubstraitToVeloxPlanValidator::validate(const
::substrait::SortRel& sortRel
if (sort.has_expr()) {
try {
auto expression = exprConverter_->toVeloxExpr(sort.expr(), rowType);
- auto expr_field = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
- if (!expr_field) {
+ auto exprField = dynamic_cast<const
core::FieldAccessTypedExpr*>(expression.get());
+ if (!exprField) {
LOG_VALIDATION_MSG("in SortRel, the sorting key in Sort Operator
only support field.");
return false;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@gluten.apache.org
For additional commands, e-mail: commits-h...@gluten.apache.org
