diff --git a/velox/core/PlanNode.h b/velox/core/PlanNode.h index f6c2eca5c917..9aec9e0cee1e 100644 --- a/velox/core/PlanNode.h +++ b/velox/core/PlanNode.h @@ -1959,16 +1959,7 @@ class UnnestNode : public PlanNode { const std::vector& unnestNames, const std::optional& ordinalityName, const PlanNodePtr& source, - bool isOuter); - - UnnestNode( - const PlanNodeId& id, - std::vector replicateVariables, - std::vector unnestVariables, - const std::vector& unnestNames, - const std::optional& ordinalityName, - const PlanNodePtr& source): - UnnestNode(id, replicateVariables, unnestVariables, unnestNames, ordinalityName, source, false) {} + bool isOuter = false); /// The order of columns in the output is: replicated columns (in the order /// specified), unnested columns (in the order specified, for maps: key diff --git a/velox/exec/Unnest.cpp b/velox/exec/Unnest.cpp index 324ee6c96d9b..ce7fd7d09ce9 100644 --- a/velox/exec/Unnest.cpp +++ b/velox/exec/Unnest.cpp @@ -103,12 +103,8 @@ RowVectorPtr Unnest::getOutput() { return nullptr; } - RowVectorPtr output = nullptr; - if (isOuter_) { - output = generateOutput(rowRange); - } else { - output = generateOutput(rowRange); - } + const auto output = isOuter_ ? generateOutput(rowRange) + : generateOutput(rowRange); if (rowRange.lastRowEnd.has_value()) { // The last row is not processed completely. @@ -255,7 +251,7 @@ VectorPtr Unnest::generateOrdinalityVector(const RowRange& range) { vector_size_t index = 0; range.forEachRow( [&](vector_size_t row, vector_size_t start, vector_size_t size) { - if constexpr(isOuter) { + if constexpr (isOuter) { if (rawOrdinalityIsNull_[row]) { ordinalityVector->setNull(index, true); } @@ -312,9 +308,8 @@ RowVectorPtr Unnest::generateOutput(const RowRange& range) { std::move(outputs)); } - template -void Unnest::countMaxNumElementsPerRow(int32_t size) { +void Unnest::countMaxNumElementsPerRow(vector_size_t numRows) { for (auto channel = 0; channel < unnestChannels_.size(); ++channel) { const auto& unnestVector = input_->childAt(unnestChannels_[channel]); @@ -337,7 +332,7 @@ void Unnest::countMaxNumElementsPerRow(int32_t size) { // Count max number of elements per row. auto* currentSizes = rawSizes_[channel]; auto* currentIndices = rawIndices_[channel]; - for (auto row = 0; row < size; ++row) { + for (auto row = 0; row < numRows; ++row) { if (!currentDecoded.isNullAt(row)) { auto unnestSize = currentSizes[currentIndices[row]]; if constexpr (isOuter) { diff --git a/velox/exec/Unnest.h b/velox/exec/Unnest.h index cef750078030..1358db52551c 100644 --- a/velox/exec/Unnest.h +++ b/velox/exec/Unnest.h @@ -108,11 +108,11 @@ class Unnest : public Operator { std::vector& outputs); // Calculate the max number of elements of each row after unnested. - // @param size The number of input rows - // @param isOuter Whether we should generate a null element - // if the array/map is null or empty then null is produced + // @param numRows The number of input rows. + // @param isOuter Whether we should generate a null element. + // if the array/map is null or empty then null is produced. template - void countMaxNumElementsPerRow(int32_t size); + void countMaxNumElementsPerRow(vector_size_t numRows); struct UnnestChannelEncoding { BufferPtr indices; diff --git a/velox/exec/tests/UnnestTest.cpp b/velox/exec/tests/UnnestTest.cpp index 78890e00e1fe..1b47f4ae29d5 100644 --- a/velox/exec/tests/UnnestTest.cpp +++ b/velox/exec/tests/UnnestTest.cpp @@ -477,26 +477,16 @@ TEST_P(UnnestTest, batchSize) { TEST_P(UnnestTest, basicArrayWithOuter) { auto vector = makeRowVector({ makeFlatVector({1, 2, 3}), - makeNullableArrayVector({ - {1, 2, std::nullopt}, - {}, - {3} - }), + makeNullableArrayVector({{1, 2, std::nullopt}, {}, {3}}), }); createDuckDbTable({vector}); - // isOuter = false - auto op1 = PlanBuilder().values({vector}).unnest({"c0"}, {"c1"}, std::nullopt, /* isOuter = */ false).planNode(); - auto params1 = makeCursorParameters(op1); - auto expected1 = makeRowVector({ - makeFlatVector({1, 1, 1, 3}), - makeNullableFlatVector({1, 2, std::nullopt, 3}), - }); - assertQuery(params1, expected1); - - // isOuter = true - auto op2 = PlanBuilder().values({vector}).unnest({"c0"}, {"c1"}, std::nullopt, /* isOuter = */ true).planNode(); + // isOuter = true. + auto op2 = PlanBuilder() + .values({vector}) + .unnest({"c0"}, {"c1"}, std::nullopt, /* isOuter = */ true) + .planNode(); auto params2 = makeCursorParameters(op2); auto expected2 = makeRowVector({ makeFlatVector({1, 1, 1, 2, 3}), @@ -504,8 +494,11 @@ TEST_P(UnnestTest, basicArrayWithOuter) { }); assertQuery(params2, expected2); - // ordinal = true && isOuter = true - auto op3 = PlanBuilder().values({vector}).unnest({"c0"}, {"c1"}, "ordinal", /* isOuter = */ true).planNode(); + // ordinal = true && isOuter = true. + auto op3 = PlanBuilder() + .values({vector}) + .unnest({"c0"}, {"c1"}, "ordinal", /* isOuter = */ true) + .planNode(); auto params3 = makeCursorParameters(op3); auto expected3 = makeRowVector({ makeFlatVector({1, 1, 1, 2, 3}),