Skip to content

Commit a2cac01

Browse files
kevinwilfongmeta-codesync[bot]
authored andcommitted
misc: Add option to IndexLookupJoinNode to enable/disable splitting of output (#17507)
Summary: Pull Request resolved: #17507 Similar to #16762 which added a per-operator splitOutput flag to UnnestNode, this adds the same option to IndexLookupJoinNode. The existing QueryConfig `index_lookup_join_split_output` applies to all IndexLookupJoin operators in the plan. This is too coarse — the decision to split output depends on downstream operators and should be made per-operator. This adds a new `splitOutput` parameter (`std::optional<bool>`) to IndexLookupJoinNode. When set, the operator uses this value instead of the QueryConfig. When unset (default), the operator falls back to the QueryConfig, preserving existing behavior. Reviewed By: xiaoxmeng Differential Revision: D105021498 fbshipit-source-id: 848510099a2789e29c39252d322b526bb27cf104
1 parent f145f40 commit a2cac01

9 files changed

Lines changed: 242 additions & 6 deletions

File tree

velox/core/PlanNode.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1820,9 +1820,35 @@ IndexLookupJoinNode::IndexLookupJoinNode(
18201820
const std::vector<IndexLookupConditionPtr>& joinConditions,
18211821
TypedExprPtr filter,
18221822
bool hasMarker,
1823+
std::optional<bool> splitOutput,
18231824
PlanNodePtr left,
18241825
TableScanNodePtr right,
18251826
RowTypePtr outputType)
1827+
: IndexLookupJoinNode(
1828+
id,
1829+
joinType,
1830+
leftKeys,
1831+
rightKeys,
1832+
joinConditions,
1833+
std::move(filter),
1834+
hasMarker,
1835+
std::move(left),
1836+
std::move(right),
1837+
std::move(outputType),
1838+
splitOutput) {}
1839+
1840+
IndexLookupJoinNode::IndexLookupJoinNode(
1841+
const PlanNodeId& id,
1842+
JoinType joinType,
1843+
const std::vector<FieldAccessTypedExprPtr>& leftKeys,
1844+
const std::vector<FieldAccessTypedExprPtr>& rightKeys,
1845+
const std::vector<IndexLookupConditionPtr>& joinConditions,
1846+
TypedExprPtr filter,
1847+
bool hasMarker,
1848+
PlanNodePtr left,
1849+
TableScanNodePtr right,
1850+
RowTypePtr outputType,
1851+
std::optional<bool> splitOutput)
18261852
: AbstractJoinNode(
18271853
id,
18281854
joinType,
@@ -1834,7 +1860,8 @@ IndexLookupJoinNode::IndexLookupJoinNode(
18341860
outputType),
18351861
lookupSourceNode_(std::move(right)),
18361862
joinConditions_(joinConditions),
1837-
hasMarker_(hasMarker) {
1863+
hasMarker_(hasMarker),
1864+
splitOutput_(splitOutput) {
18381865
VELOX_USER_CHECK(
18391866
!leftKeys.empty(),
18401867
"The index lookup join node requires at least one join key");
@@ -1921,6 +1948,11 @@ PlanNodePtr IndexLookupJoinNode::create(
19211948

19221949
const bool hasMarker = obj["hasMarker"].asBool();
19231950

1951+
std::optional<bool> splitOutput = std::nullopt;
1952+
if (obj.count("splitOutput")) {
1953+
splitOutput = obj["splitOutput"].asBool();
1954+
}
1955+
19241956
auto outputType = deserializeRowType(obj["outputType"]);
19251957

19261958
return std::make_shared<IndexLookupJoinNode>(
@@ -1931,6 +1963,7 @@ PlanNodePtr IndexLookupJoinNode::create(
19311963
std::move(joinConditions),
19321964
filter,
19331965
hasMarker,
1966+
splitOutput,
19341967
sources[0],
19351968
std::move(lookupSource),
19361969
std::move(outputType));
@@ -1949,6 +1982,9 @@ folly::dynamic IndexLookupJoinNode::serialize() const {
19491982
obj["filter"] = filter_->serialize();
19501983
}
19511984
obj["hasMarker"] = hasMarker_;
1985+
if (splitOutput_.has_value()) {
1986+
obj["splitOutput"] = splitOutput_.value();
1987+
}
19521988
return obj;
19531989
}
19541990

velox/core/PlanNode.h

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3711,6 +3711,26 @@ class IndexLookupJoinNode : public AbstractJoinNode {
37113711
bool hasMarker,
37123712
PlanNodePtr left,
37133713
TableScanNodePtr right,
3714+
RowTypePtr outputType,
3715+
std::optional<bool> splitOutput = std::nullopt);
3716+
3717+
/// @param splitOutput Optional flag to control whether the operator should
3718+
/// split output batches if they are too large. If true, output is split into
3719+
/// batches according to Operator's outputBatchRows logic. If false, output is
3720+
/// not split and output batches match input batches 1:1. If not set, defaults
3721+
/// to the value of the index_lookup_join_split_output config in the
3722+
/// QueryConfig.
3723+
IndexLookupJoinNode(
3724+
const PlanNodeId& id,
3725+
JoinType joinType,
3726+
const std::vector<FieldAccessTypedExprPtr>& leftKeys,
3727+
const std::vector<FieldAccessTypedExprPtr>& rightKeys,
3728+
const std::vector<IndexLookupConditionPtr>& joinConditions,
3729+
TypedExprPtr filter,
3730+
bool hasMarker,
3731+
std::optional<bool> splitOutput,
3732+
PlanNodePtr left,
3733+
TableScanNodePtr right,
37143734
RowTypePtr outputType);
37153735

37163736
class Builder
@@ -3723,6 +3743,7 @@ class IndexLookupJoinNode : public AbstractJoinNode {
37233743
joinConditions_ = other.joinConditions();
37243744
filter_ = other.filter();
37253745
hasMarker_ = other.hasMarker();
3746+
splitOutput_ = other.splitOutput();
37263747
}
37273748

37283749
/// Set lookup conditions for index lookup that can't be converted into
@@ -3745,6 +3766,13 @@ class IndexLookupJoinNode : public AbstractJoinNode {
37453766
return *this;
37463767
}
37473768

3769+
/// Set whether to split large output into multiple batches, std::nullopt
3770+
/// means respect index_lookup_join_split_output in the QueryConfig.
3771+
Builder& splitOutput(std::optional<bool> splitOutput) {
3772+
splitOutput_ = splitOutput;
3773+
return *this;
3774+
}
3775+
37483776
std::shared_ptr<IndexLookupJoinNode> build() const {
37493777
VELOX_USER_CHECK(id_.has_value(), "IndexLookupJoinNode id is not set");
37503778
VELOX_USER_CHECK(
@@ -3768,6 +3796,7 @@ class IndexLookupJoinNode : public AbstractJoinNode {
37683796
joinConditions_,
37693797
filter_.value_or(nullptr),
37703798
hasMarker_,
3799+
splitOutput_,
37713800
left_.value(),
37723801
std::dynamic_pointer_cast<const TableScanNode>(right_.value()),
37733802
outputType_.value());
@@ -3776,6 +3805,7 @@ class IndexLookupJoinNode : public AbstractJoinNode {
37763805
private:
37773806
std::vector<IndexLookupConditionPtr> joinConditions_;
37783807
bool hasMarker_{false};
3808+
std::optional<bool> splitOutput_;
37793809
};
37803810

37813811
bool supportsBarrier() const override {
@@ -3805,6 +3835,10 @@ class IndexLookupJoinNode : public AbstractJoinNode {
38053835
return hasMarker_;
38063836
}
38073837

3838+
const std::optional<bool>& splitOutput() const {
3839+
return splitOutput_;
3840+
}
3841+
38083842
void accept(const PlanNodeVisitor& visitor, PlanNodeVisitorContext& context)
38093843
const override;
38103844

@@ -3828,6 +3862,10 @@ class IndexLookupJoinNode : public AbstractJoinNode {
38283862

38293863
/// Whether to include a marker column for left joins to indicate matches.
38303864
const bool hasMarker_;
3865+
3866+
/// Optional flag to control whether to split output batches. When set,
3867+
/// overrides the index_lookup_join_split_output QueryConfig.
3868+
const std::optional<bool> splitOutput_;
38313869
};
38323870

38333871
using IndexLookupJoinNodePtr = std::shared_ptr<const IndexLookupJoinNode>;

velox/core/QueryConfig.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1335,7 +1335,9 @@ class QueryConfig {
13351335
0,
13361336
"Maximum input batches to prefetch for index lookup. 0 disables.")
13371337

1338-
/// If true, index join operator may split output per input batch.
1338+
/// If true, index join operator may split output per input batch. This can be
1339+
/// overridden on a per operator basis by the splitOutput parameter in the
1340+
/// IndexLookupJoinNode.
13391341
VELOX_QUERY_CONFIG(
13401342
kIndexLookupJoinSplitOutput,
13411343
indexLookupJoinSplitOutput,

velox/core/tests/PlanNodeBuilderTest.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,7 @@ TEST_F(PlanNodeBuilderTest, indexLookupJoinNode) {
776776
.assignments({{"c1", std::make_shared<DummyColumnHandle>()}})
777777
.build();
778778
const auto outputType = ROW({"c0"}, {BIGINT()});
779+
std::optional<bool> splitOutput = true;
779780

780781
const auto verify =
781782
[&](const std::shared_ptr<const IndexLookupJoinNode>& node) {
@@ -790,6 +791,7 @@ TEST_F(PlanNodeBuilderTest, indexLookupJoinNode) {
790791
EXPECT_EQ(node->sources()[0], left);
791792
EXPECT_EQ(node->sources()[1], right);
792793
EXPECT_EQ(node->outputType(), outputType);
794+
EXPECT_EQ(node->splitOutput(), splitOutput);
793795
};
794796

795797
const auto node = IndexLookupJoinNode::Builder()
@@ -801,6 +803,7 @@ TEST_F(PlanNodeBuilderTest, indexLookupJoinNode) {
801803
.left(left)
802804
.right(right)
803805
.outputType(outputType)
806+
.splitOutput(splitOutput)
804807
.build();
805808
verify(node);
806809

velox/exec/IndexLookupJoin.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,16 @@ IndexLookupJoin::IndexLookupJoin(
302302
operatorId,
303303
joinNode->id(),
304304
OperatorType::kIndexLookupJoin),
305-
splitOutput_{driverCtx->queryConfig().indexLookupJoinSplitOutput()},
305+
splitOutput_{
306+
(joinNode->splitOutput().has_value() &&
307+
joinNode->splitOutput().value()) ||
308+
(!joinNode->splitOutput().has_value() &&
309+
driverCtx->queryConfig().indexLookupJoinSplitOutput())},
306310
// TODO: support to update output batch size with output size stats during
307311
// the lookup processing.
308312
outputBatchSize_{
309-
driverCtx->queryConfig().indexLookupJoinSplitOutput()
310-
? outputBatchRows()
311-
: std::numeric_limits<vector_size_t>::max()},
313+
splitOutput_ ? outputBatchRows()
314+
: std::numeric_limits<vector_size_t>::max()},
312315
joinType_{joinNode->joinType()},
313316
hasMarker_(joinNode->hasMarker()),
314317
probeType_{joinNode->sources()[0]->outputType()},

velox/exec/tests/IndexLookupJoinTest.cpp

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,28 @@ TEST_P(IndexLookupJoinTest, planNodeAndSerde) {
492492
testSerde(plan);
493493
}
494494

495+
// with splitOutput.
496+
for (const auto splitOutput :
497+
{std::optional<bool>(true),
498+
std::optional<bool>(false),
499+
std::optional<bool>(std::nullopt)}) {
500+
auto plan = PlanBuilder(planNodeIdGenerator)
501+
.values({left})
502+
.startIndexLookupJoin()
503+
.leftKeys({"t0"})
504+
.rightKeys({"u0"})
505+
.indexSource(indexTableScan)
506+
.outputLayout({"t0", "u1", "t2", "t1"})
507+
.joinType(core::JoinType::kInner)
508+
.splitOutput(splitOutput)
509+
.endIndexLookupJoin()
510+
.planNode();
511+
auto indexLookupJoinNode =
512+
std::dynamic_pointer_cast<const core::IndexLookupJoinNode>(plan);
513+
ASSERT_EQ(indexLookupJoinNode->splitOutput(), splitOutput);
514+
testSerde(plan);
515+
}
516+
495517
// bad join type.
496518
{
497519
VELOX_ASSERT_USER_THROW(
@@ -2664,6 +2686,130 @@ TEST_P(IndexLookupJoinTest, outputBatchSizeWithLeftJoin) {
26642686
}
26652687
}
26662688

2689+
TEST_P(IndexLookupJoinTest, splitOutputNodeOverride) {
2690+
IndexTableData tableData;
2691+
generateIndexTableData({3'000, 1, 1}, tableData, pool_);
2692+
2693+
const int numProbeBatches = 10;
2694+
const int numRowsPerProbeBatch = 100;
2695+
const int maxBatchRows = 10;
2696+
2697+
const auto probeVectors = generateProbeInput(
2698+
numProbeBatches,
2699+
numRowsPerProbeBatch,
2700+
1,
2701+
tableData,
2702+
pool_,
2703+
{"t0", "t1", "t2"},
2704+
GetParam().hasNullKeys,
2705+
{},
2706+
{},
2707+
/*equalMatchPct=*/100);
2708+
std::vector<std::shared_ptr<TempFilePath>> probeFiles =
2709+
createProbeFiles(probeVectors);
2710+
2711+
createDuckDbTable("t", probeVectors);
2712+
createDuckDbTable("u", {tableData.tableVectors});
2713+
2714+
const auto indexTable = TestIndexTable::create(
2715+
/*numEqualJoinKeys=*/3,
2716+
tableData.keyVectors,
2717+
tableData.valueVectors,
2718+
*pool());
2719+
const auto indexTableHandle = makeIndexTableHandle(
2720+
indexTable, GetParam().asyncLookup, GetParam().needsIndexSplit);
2721+
2722+
struct {
2723+
std::optional<bool> nodeSplitOutput;
2724+
bool configSplitOutput;
2725+
bool expectSplit;
2726+
2727+
std::string debugString() const {
2728+
return fmt::format(
2729+
"nodeSplitOutput: {}, configSplitOutput: {}, expectSplit: {}",
2730+
nodeSplitOutput.has_value()
2731+
? (nodeSplitOutput.value() ? "true" : "false")
2732+
: "nullopt",
2733+
configSplitOutput,
2734+
expectSplit);
2735+
}
2736+
} testSettings[] = {
2737+
// Node splitOutput not set, use config.
2738+
{std::nullopt, true, true},
2739+
{std::nullopt, false, false},
2740+
// Node splitOutput=true overrides config.
2741+
{true, true, true},
2742+
{true, false, true},
2743+
// Node splitOutput=false overrides config.
2744+
{false, true, false},
2745+
{false, false, false},
2746+
};
2747+
2748+
for (const auto& testData : testSettings) {
2749+
SCOPED_TRACE(testData.debugString());
2750+
2751+
auto planNodeIdGenerator = std::make_shared<core::PlanNodeIdGenerator>();
2752+
2753+
const auto indexScanNode = makeIndexScanNode(
2754+
planNodeIdGenerator,
2755+
indexTableHandle,
2756+
makeScanOutputType({"u0", "u1", "u2", "u5"}),
2757+
makeIndexColumnHandles({"u0", "u1", "u2", "u5"}));
2758+
2759+
core::PlanNodeId joinNodeId;
2760+
auto plan = PlanBuilder(planNodeIdGenerator, pool_.get())
2761+
.startTableScan()
2762+
.outputType(probeType_)
2763+
.endTableScan()
2764+
.captureScanNodeId(probeScanNodeId_)
2765+
.startIndexLookupJoin()
2766+
.leftKeys({"t0", "t1", "t2"})
2767+
.rightKeys({"u0", "u1", "u2"})
2768+
.indexSource(indexScanNode)
2769+
.outputLayout({"t4", "u5"})
2770+
.joinType(core::JoinType::kInner)
2771+
.splitOutput(testData.nodeSplitOutput)
2772+
.endIndexLookupJoin()
2773+
.capturePlanNodeId(joinNodeId)
2774+
.planNode();
2775+
2776+
const int expectedNumOutputVectors = testData.expectSplit
2777+
? ((numRowsPerProbeBatch + maxBatchRows - 1) / maxBatchRows) *
2778+
numProbeBatches
2779+
: numProbeBatches;
2780+
2781+
AssertQueryBuilder queryBuilder(duckDbQueryRunner_);
2782+
queryBuilder.plan(plan)
2783+
.config(
2784+
core::QueryConfig::kIndexLookupJoinMaxPrefetchBatches,
2785+
std::to_string(GetParam().numPrefetches))
2786+
.config(
2787+
core::QueryConfig::kPreferredOutputBatchRows,
2788+
std::to_string(maxBatchRows))
2789+
.config(
2790+
core::QueryConfig::kPreferredOutputBatchBytes,
2791+
std::to_string(1ULL << 30))
2792+
.config(
2793+
core::QueryConfig::kIndexLookupJoinSplitOutput,
2794+
testData.configSplitOutput ? "true" : "false")
2795+
.splits(probeScanNodeId_, makeHiveConnectorSplits(probeFiles))
2796+
.serialExecution(GetParam().serialExecution)
2797+
.barrierExecution(GetParam().serialExecution);
2798+
if (GetParam().needsIndexSplit) {
2799+
queryBuilder.split(
2800+
indexScanNodeId_,
2801+
Split(
2802+
std::make_shared<TestIndexConnectorSplit>(
2803+
kTestIndexConnectorName)));
2804+
}
2805+
const auto task = queryBuilder.assertResults(
2806+
"SELECT t.c4, u.c5 FROM t, u WHERE t.c0 = u.c0 AND t.c1 = u.c1 AND t.c2 = u.c2");
2807+
ASSERT_EQ(
2808+
toPlanStats(task->taskStats()).at(joinNodeId).outputVectors,
2809+
expectedNumOutputVectors);
2810+
}
2811+
}
2812+
26672813
DEBUG_ONLY_TEST_P(IndexLookupJoinTest, statsSplitter) {
26682814
IndexTableData tableData;
26692815
generateIndexTableData({100, 1, 1}, tableData, pool_);

velox/exec/tests/utils/PlanBuilder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2725,6 +2725,7 @@ core::PlanNodePtr PlanBuilder::IndexLookupJoinBuilder::build(
27252725
std::move(joinConditionPtrs),
27262726
filterExpr,
27272727
hasMarker_,
2728+
splitOutput_,
27282729
std::move(planBuilder_.planNode_),
27292730
indexSource_,
27302731
std::move(outputType));

0 commit comments

Comments
 (0)