Skip to content

Commit

Permalink
Improve output of WindowNode::toString (#7756)
Browse files Browse the repository at this point in the history 
Summary:
Improve the output of WindowNode::toString:

- Remove inputsSorted [0] suffix
- Replace inputsSorted [1] suffix with STREAMING prefix

Pull Request resolved: #7756

Reviewed By: xiaoxmeng

Differential Revision: D51608357

Pulled By: mbasmanova

fbshipit-source-id: 3e5588cac7d0efd525f0591adc59aa2e847ef77c
  • Loading branch information
@mbasmanova @facebook-github-bot
mbasmanova authored and facebook-github-bot committed Nov 28, 2023
1 parent b30c094 commit a9846b9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 27 deletions.
29 changes: 16 additions & 13 deletions velox/core/PlanNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1243,25 +1243,28 @@ WindowNode::WindowNode(
}

void WindowNode::addDetails(std::stringstream& stream) const {
stream << "partition by [";
if (inputsSorted_) {
stream << "STREAMING ";
}

if (!partitionKeys_.empty()) {
stream << "partition by [";
addFields(stream, partitionKeys_);
stream << "] ";
}
stream << "] ";

stream << "order by [";
addSortingKeys(sortingKeys_, sortingOrders_, stream);
stream << "] ";

auto numInputCols = sources_[0]->outputType()->size();
auto numOutputCols = outputType_->size();
for (auto i = numInputCols; i < numOutputCols; i++) {
appendComma(i - numInputCols, stream);
stream << outputType_->names()[i] << " := ";
addWindowFunction(stream, windowFunctions_[i - numInputCols]);
if (!sortingKeys_.empty()) {
stream << "order by [";
addSortingKeys(sortingKeys_, sortingOrders_, stream);
stream << "] ";
}

stream << " inputsSorted [" << inputsSorted_ << "]";
const auto numInputs = inputType()->size();
for (auto i = 0; i < windowFunctions_.size(); i++) {
appendComma(i, stream);
stream << outputType_->names()[i + numInputs] << " := ";
addWindowFunction(stream, windowFunctions_[i]);
}
}

namespace {
Expand Down
11 changes: 5 additions & 6 deletions velox/exec/tests/PlanBuilderTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ TEST_F(PlanBuilderTest, windowFunctionCall) {
.planNode()
->toString(true, false),
"-- Window[partition by [a] order by [b ASC NULLS LAST] "
"d := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW inputsSorted [0]] "
"d := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, d:BIGINT\n");

VELOX_CHECK_EQ(
Expand All @@ -124,8 +124,8 @@ TEST_F(PlanBuilderTest, windowFunctionCall) {
.window({"window1(c) over (partition by a) as d"})
.planNode()
->toString(true, false),
"-- Window[partition by [a] order by [] "
"d := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW inputsSorted [0]] "
"-- Window[partition by [a] "
"d := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, d:BIGINT\n");

VELOX_CHECK_EQ(
Expand All @@ -134,8 +134,7 @@ TEST_F(PlanBuilderTest, windowFunctionCall) {
.window({"window1(c) over ()"})
.planNode()
->toString(true, false),
"-- Window[partition by [] order by [] "
"w0 := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW inputsSorted [0]] "
"-- Window[w0 := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and CURRENT ROW] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, w0:BIGINT\n");

VELOX_ASSERT_THROW(
Expand Down Expand Up @@ -184,7 +183,7 @@ TEST_F(PlanBuilderTest, windowFrame) {
"d7 := window1(ROW[\"c\"]) ROWS between CURRENT ROW and UNBOUNDED FOLLOWING, "
"d8 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and UNBOUNDED FOLLOWING, "
"d9 := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING, "
"d10 := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING inputsSorted [0]] "
"d10 := window1(ROW[\"c\"]) RANGE between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, d1:BIGINT, d2:BIGINT, d3:BIGINT, d4:BIGINT, "
"d5:BIGINT, d6:BIGINT, d7:BIGINT, d8:BIGINT, d9:BIGINT, d10:BIGINT\n");

Expand Down
28 changes: 20 additions & 8 deletions velox/exec/tests/PlanNodeToStringTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ TEST_F(PlanNodeToStringTest, window) {
ASSERT_EQ("-- Window\n", plan->toString());
ASSERT_EQ(
"-- Window[partition by [a] order by [b ASC NULLS LAST] "
"d := window1(ROW[\"c\"]) RANGE between 10 PRECEDING and UNBOUNDED FOLLOWING inputsSorted [0]] "
"d := window1(ROW[\"c\"]) RANGE between 10 PRECEDING and UNBOUNDED FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, d:BIGINT\n",
plan->toString(true, false));

Expand All @@ -725,8 +725,20 @@ TEST_F(PlanNodeToStringTest, window) {
.planNode();
ASSERT_EQ("-- Window\n", plan->toString());
ASSERT_EQ(
"-- Window[partition by [a] order by [] "
"w0 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and b FOLLOWING inputsSorted [0]] "
"-- Window[partition by [a] "
"w0 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and b FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, w0:BIGINT\n",
plan->toString(true, false));

plan = PlanBuilder()
.tableScan(ROW({"a", "b", "c"}, {VARCHAR(), BIGINT(), BIGINT()}))
.window({"window1(c) over (order by a "
"range between current row and b following)"})
.planNode();
ASSERT_EQ("-- Window\n", plan->toString());
ASSERT_EQ(
"-- Window[order by [a ASC NULLS LAST] "
"w0 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and b FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, w0:BIGINT\n",
plan->toString(true, false));

Expand All @@ -738,8 +750,8 @@ TEST_F(PlanNodeToStringTest, window) {
.planNode();
ASSERT_EQ("-- Window\n", plan->toString());
ASSERT_EQ(
"-- Window[partition by [a] order by [b ASC NULLS LAST] "
"d := window1(ROW[\"c\"]) RANGE between 10 PRECEDING and UNBOUNDED FOLLOWING inputsSorted [1]] "
"-- Window[STREAMING partition by [a] order by [b ASC NULLS LAST] "
"d := window1(ROW[\"c\"]) RANGE between 10 PRECEDING and UNBOUNDED FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, d:BIGINT\n",
plan->toString(true, false));

Expand All @@ -750,8 +762,8 @@ TEST_F(PlanNodeToStringTest, window) {
.planNode();
ASSERT_EQ("-- Window\n", plan->toString());
ASSERT_EQ(
"-- Window[partition by [a] order by [] "
"w0 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and b FOLLOWING inputsSorted [1]] "
"-- Window[STREAMING partition by [a] "
"w0 := window1(ROW[\"c\"]) RANGE between CURRENT ROW and b FOLLOWING] "
"-> a:VARCHAR, b:BIGINT, c:BIGINT, w0:BIGINT\n",
plan->toString(true, false));
}
Expand All @@ -766,7 +778,7 @@ TEST_F(PlanNodeToStringTest, rowNumber) {
"-- RowNumber[] -> a:VARCHAR, row_number:BIGINT\n",
plan->toString(true, false));

// Dont' emit row number.
// Don't emit row number.
plan = PlanBuilder()
.tableScan(ROW({"a"}, {VARCHAR()}))
.rowNumber({}, std::nullopt, false)
Expand Down

0 comments on commit a9846b9

Please sign in to comment.