From a9846b9ce029ff1375b6bb47c7ee2071e11e267d Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Tue, 28 Nov 2023 06:29:58 -0800 Subject: [PATCH] Improve output of WindowNode::toString (#7756) Summary: Improve the output of WindowNode::toString: - Remove inputsSorted [0] suffix - Replace inputsSorted [1] suffix with STREAMING prefix Pull Request resolved: https://github.com/facebookincubator/velox/pull/7756 Reviewed By: xiaoxmeng Differential Revision: D51608357 Pulled By: mbasmanova fbshipit-source-id: 3e5588cac7d0efd525f0591adc59aa2e847ef77c --- velox/core/PlanNode.cpp | 29 +++++++++++++---------- velox/exec/tests/PlanBuilderTest.cpp | 11 ++++----- velox/exec/tests/PlanNodeToStringTest.cpp | 28 +++++++++++++++------- 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/velox/core/PlanNode.cpp b/velox/core/PlanNode.cpp index 6cf42dbb95e0..494d681dbd88 100644 --- a/velox/core/PlanNode.cpp +++ b/velox/core/PlanNode.cpp @@ -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 { diff --git a/velox/exec/tests/PlanBuilderTest.cpp b/velox/exec/tests/PlanBuilderTest.cpp index 871143eaf511..670593fa6883 100644 --- a/velox/exec/tests/PlanBuilderTest.cpp +++ b/velox/exec/tests/PlanBuilderTest.cpp @@ -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( @@ -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( @@ -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( @@ -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"); diff --git a/velox/exec/tests/PlanNodeToStringTest.cpp b/velox/exec/tests/PlanNodeToStringTest.cpp index 790f6a3ca056..3a344d2a9fb7 100644 --- a/velox/exec/tests/PlanNodeToStringTest.cpp +++ b/velox/exec/tests/PlanNodeToStringTest.cpp @@ -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)); @@ -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)); @@ -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)); @@ -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)); } @@ -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)