Skip to content

Commit

Permalink
Make PlanNode enum to string conversions code consistent (#11144)
Browse files Browse the repository at this point in the history
Summary:
PartitionedOutputNode and LocalPartitionNode enum to string conversions were not consistent with how the enum to string conversions are done for rest of the PlanNodes enum fields.

Pull Request resolved: #11144

Reviewed By: Yuhta

Differential Revision: D63764489

Pulled By: xiaoxmeng

fbshipit-source-id: a1812b25556c5b9b079e4b4279c135db5b01e3b3
  • Loading branch information
aditi-pandit authored and facebook-github-bot committed Oct 2, 2024
1 parent 0dc5609 commit b6a7cd2
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 34 deletions.
54 changes: 23 additions & 31 deletions velox/core/PlanNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1971,17 +1971,6 @@ PlanNodePtr LocalPartitionNode::create(
deserializeSources(obj, context));
}

// static
const char* LocalPartitionNode::typeName(Type type) {
switch (type) {
case Type::kGather:
return "GATHER";
case Type::kRepartition:
return "REPARTITION";
}
VELOX_UNREACHABLE();
}

namespace {
std::unordered_map<LocalPartitionNode::Type, std::string>
localPartitionTypeNames() {
Expand All @@ -1992,6 +1981,12 @@ localPartitionTypeNames() {
}
} // namespace

// static
const char* LocalPartitionNode::typeName(Type type) {
static const auto kLocalPartitionTypeNames = localPartitionTypeNames();
return kLocalPartitionTypeNames.at(type).c_str();
}

// static
LocalPartitionNode::Type LocalPartitionNode::typeFromName(
const std::string& name) {
Expand All @@ -2016,32 +2011,29 @@ PlanNodePtr EnforceSingleRowNode::create(
deserializePlanNodeId(obj), deserializeSingleSource(obj, context));
}

namespace {
std::unordered_map<PartitionedOutputNode::Kind, std::string>
partitionKindNames() {
return {
{PartitionedOutputNode::Kind::kPartitioned, "PARTITIONED"},
{PartitionedOutputNode::Kind::kBroadcast, "BROADCAST"},
{PartitionedOutputNode::Kind::kArbitrary, "ARBITRARY"},
};
}

} // namespace

// static
std::string PartitionedOutputNode::kindString(Kind kind) {
switch (kind) {
case Kind::kPartitioned:
return "PARTITIONED";
case Kind::kBroadcast:
return "BROADCAST";
case Kind::kArbitrary:
return "ARBITRARY";
default:
return fmt::format("INVALID OUTPUT KIND {}", static_cast<int>(kind));
}
static const auto kPartitionNames = partitionKindNames();
return kPartitionNames.at(kind);
}

// static
PartitionedOutputNode::Kind PartitionedOutputNode::stringToKind(
std::string str) {
if (str == "PARTITIONED") {
return Kind::kPartitioned;
} else if (str == "BROADCAST") {
return Kind::kBroadcast;
} else if (str == "ARBITRARY") {
return Kind::kArbitrary;
} else {
VELOX_FAIL("Unknown output buffer type: {}", str);
}
const std::string& name) {
static const auto kPartitionKinds = invertMap(partitionKindNames());
return kPartitionKinds.at(name);
}

void PartitionedOutputNode::addDetails(std::stringstream& stream) const {
Expand Down
2 changes: 1 addition & 1 deletion velox/core/PlanNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ class PartitionedOutputNode : public PlanNode {
kArbitrary,
};
static std::string kindString(Kind kind);
static Kind stringToKind(std::string str);
static Kind stringToKind(const std::string& str);

PartitionedOutputNode(
const PlanNodeId& id,
Expand Down
4 changes: 2 additions & 2 deletions velox/exec/tests/OutputBufferManagerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,10 +535,10 @@ TEST_F(OutputBufferManagerTest, outputType) {
PartitionedOutputNode::kindString(
PartitionedOutputNode::Kind::kBroadcast),
"BROADCAST");
ASSERT_EQ(
EXPECT_THROW(
PartitionedOutputNode::kindString(
static_cast<PartitionedOutputNode::Kind>(100)),
"INVALID OUTPUT KIND 100");
std::out_of_range);
}

TEST_F(OutputBufferManagerTest, destinationBuffer) {
Expand Down

0 comments on commit b6a7cd2

Please sign in to comment.