From 0c3bde68d4703a47fb52a5ec1bf5fb7a2bc6f204 Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Thu, 27 Aug 2026 07:47:19 -0700 Subject: [PATCH 1/2] test(optimizer): Run PlanTest under both optimizers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: `PlanTest` ran under v1 only. It now runs under both, so every query in the suite has to plan under v2 as well. Half of the plan assertions hold unchanged under both optimizers and stay in `AXIOM_ASSERT_PLAN`. The 10 where v2 produces a different shape use `AXIOM_ASSERT_PLAN_V1`: the query still has to plan under v2, but its shape is not compared. The shapes v2 does not match today: - Subfield-filter pushdown into the scan. - Filters left after constant folding. - `ParallelProject` — v2 emits a single `Project`. Differential Revision: D117696681 --- axiom/optimizer/tests/PlanTest.cpp | 58 +++++++++++++++++------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/axiom/optimizer/tests/PlanTest.cpp b/axiom/optimizer/tests/PlanTest.cpp index 1adcfc5a0..3060cb5f3 100644 --- a/axiom/optimizer/tests/PlanTest.cpp +++ b/axiom/optimizer/tests/PlanTest.cpp @@ -31,7 +31,8 @@ namespace { using namespace facebook::velox; namespace lp = facebook::axiom::logical_plan; -class PlanTest : public test::HiveQueriesTestBase { +class PlanTest : public test::HiveQueriesTestBase, + public ::testing::WithParamInterface { protected: static void SetUpTestCase() { test::HiveQueriesTestBase::SetUpTestCase(); @@ -43,6 +44,11 @@ class PlanTest : public test::HiveQueriesTestBase { test::registerDfFunctions(); } + void SetUp() override { + useV2_ = GetParam(); + test::HiveQueriesTestBase::SetUp(); + } + lp::PlanBuilder::Context makeContext() const { return lp::PlanBuilder::Context{kTestConnectorId, kDefaultSchema}; } @@ -54,7 +60,7 @@ class PlanTest : public test::HiveQueriesTestBase { } }; -TEST_F(PlanTest, dedupEmptyArrays) { +TEST_P(PlanTest, dedupEmptyArrays) { auto logicalPlan = lp::PlanBuilder() .values(ROW({}), {variant::row({})}) @@ -80,7 +86,7 @@ TEST_F(PlanTest, dedupEmptyArrays) { // Verify that optimizer can handle connectors that do not support filter // pushdown. -TEST_F(PlanTest, rejectedFilters) { +TEST_P(PlanTest, rejectedFilters) { const auto mapType = MAP(BIGINT(), DOUBLE()); testConnector_->addTable( "t", ROW({"a", "b", "c"}, {BIGINT(), DOUBLE(), mapType})); @@ -141,7 +147,7 @@ TEST_F(PlanTest, rejectedFilters) { .project() // project c.x + 1, c.y + 2, c.z + 3 .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } // SELECT 1 FROM t WHERE c.x > 10. @@ -162,7 +168,7 @@ TEST_F(PlanTest, rejectedFilters) { .project({"1"}) .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } // SELECT c.y + 1 FROM t WHERE c.x > 10. @@ -184,11 +190,11 @@ TEST_F(PlanTest, rejectedFilters) { .project() // project c.y + 1 .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } } -TEST_F(PlanTest, specialFormConstantFold) { +TEST_P(PlanTest, specialFormConstantFold) { testConnector_->addTable("numbers", ROW({"a", "b", "c"}, BIGINT())); struct TestCase { @@ -234,7 +240,7 @@ TEST_F(PlanTest, specialFormConstantFold) { } auto plan = toSingleNodePlan(logicalPlan); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } std::vector projectTestCases = { @@ -284,7 +290,7 @@ TEST_F(PlanTest, specialFormConstantFold) { // Verifies that func(..., null, ...) is folded to null for // default-null-behavior functions and is not folded for // non-default-null-behavior functions. -TEST_F(PlanTest, nullPropagation) { +TEST_P(PlanTest, nullPropagation) { testConnector_->addTable("t", ROW({"a", "b", "c"}, BIGINT())); struct TestCase { @@ -314,11 +320,11 @@ TEST_F(PlanTest, nullPropagation) { auto matcher = matchScan("t").project({expected, "a", "b"}).build(); auto plan = toSingleNodePlan(logicalPlan); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } } -TEST_F(PlanTest, inList) { +TEST_P(PlanTest, inList) { testConnector_->addTable( "numbers", ROW({"a", "b", "c"}, {BIGINT(), DOUBLE(), VARCHAR()})); @@ -344,7 +350,7 @@ TEST_F(PlanTest, inList) { auto matcher = scanMatcher().filter("false").project().build(); auto plan = toSingleNodePlan(logicalPlan); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } { auto logicalPlan = @@ -367,7 +373,7 @@ TEST_F(PlanTest, inList) { } } -TEST_F(PlanTest, multipleConnectors) { +TEST_P(PlanTest, multipleConnectors) { auto extraConnector = std::make_shared("extra"); velox::connector::registerConnector(extraConnector); connector::ConnectorMetadataRegistry::global().insert( @@ -397,7 +403,7 @@ TEST_F(PlanTest, multipleConnectors) { AXIOM_ASSERT_PLAN(plan, matcher); } -TEST_F(PlanTest, filterToJoinEdge) { +TEST_P(PlanTest, filterToJoinEdge) { auto nationType = ROW({"n_regionkey"}, BIGINT()); auto regionType = ROW({"r_regionkey"}, BIGINT()); @@ -458,13 +464,13 @@ TEST_F(PlanTest, filterToJoinEdge) { .project() .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } checkSame(logicalPlan, referencePlan); } -TEST_F(PlanTest, filterBreakup) { +TEST_P(PlanTest, filterBreakup) { const char* filterText = " (\n" " l_partkey = p_partkey\n" @@ -531,7 +537,7 @@ TEST_F(PlanTest, filterBreakup) { .singleAggregation() .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } auto referenceBuilder = @@ -543,7 +549,7 @@ TEST_F(PlanTest, filterBreakup) { checkSame(logicalPlan, referencePlan); } -TEST_F(PlanTest, values) { +TEST_P(PlanTest, values) { auto nationType = ROW({"n_nationkey", "n_regionkey", "n_name", "n_comment"}, {BIGINT(), BIGINT(), VARCHAR(), VARCHAR()}); @@ -779,7 +785,7 @@ TEST_F(PlanTest, values) { } } -TEST_F(PlanTest, parallelCse) { +TEST_P(PlanTest, parallelCse) { testConnector_->addTable("t", ROW({"a", "b", "c"}, INTEGER())); { @@ -801,7 +807,7 @@ TEST_F(PlanTest, parallelCse) { .project() .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } { @@ -823,11 +829,11 @@ TEST_F(PlanTest, parallelCse) { .project() .build(); - AXIOM_ASSERT_PLAN(plan, matcher); + AXIOM_ASSERT_PLAN_V1(plan, matcher); } } -TEST_F(PlanTest, lastProjection) { +TEST_P(PlanTest, lastProjection) { testConnector_->addTable( "numbers", ROW({"a", "b", "c"}, {BIGINT(), DOUBLE(), VARCHAR()})); @@ -848,7 +854,7 @@ TEST_F(PlanTest, lastProjection) { AXIOM_ASSERT_PLAN(plan, matcher); } -TEST_F(PlanTest, orderByDuplicateKeys) { +TEST_P(PlanTest, orderByDuplicateKeys) { testConnector_->addTable("t", ROW({"a"}, {BIGINT()})); auto logicalPlan = lp::PlanBuilder(makeContext()) @@ -867,7 +873,7 @@ TEST_F(PlanTest, orderByDuplicateKeys) { AXIOM_ASSERT_PLAN(plan, matcher); } -TEST_F(PlanTest, lambdaArgs) { +TEST_P(PlanTest, lambdaArgs) { testConnector_->addTable( "t", ROW({"a", "b"}, {ARRAY(ARRAY(REAL())), BIGINT()})); @@ -886,7 +892,7 @@ TEST_F(PlanTest, lambdaArgs) { AXIOM_ASSERT_PLAN(plan, matcher); } -TEST_F(PlanTest, outputNames) { +TEST_P(PlanTest, outputNames) { testConnector_->addTable("t", ROW({"a", "b"}, {BIGINT(), BIGINT()})); auto test = [&](std::string_view sql, @@ -907,6 +913,8 @@ TEST_F(PlanTest, outputNames) { test(R"(SELECT a AS "", b FROM t)", {"", "b"}); } +AXIOM_INSTANTIATE_V1_V2(PlanTest); + } // namespace } // namespace facebook::axiom::optimizer From dc6778016819a55f8dc74a1e7222a68a98a6a72e Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Thu, 27 Aug 2026 08:01:22 -0700 Subject: [PATCH 2/2] test(optimizer): Dual-run UnknownStatsJoinTest Summary: `UnknownStatsJoinTest` ran under v1 only. It now runs under both, so every query in the suite has to plan under v2 as well. 9 of the 12 plan assertions hold under both optimizers unchanged. `twoJoins` needed a `filterIf(!useV2_)`: v2 folds the `rand() < 0.1` filter into the join instead of emitting a separate `Filter`, and the join order the test is about is unaffected. The remaining three use `AXIOM_ASSERT_PLAN_V1` and record a real gap. Two large tables that join only through a third one should hash-join through it. With `syntacticJoinOrder = false`, v1 does that; v2 cross-joins the two and hash-joins the third afterwards, which is the plan v1 produces only when the syntactic order is pinned. bypass-github-export-checks Differential Revision: D117697551 --- .../optimizer/tests/UnknownStatsJoinTest.cpp | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/axiom/optimizer/tests/UnknownStatsJoinTest.cpp b/axiom/optimizer/tests/UnknownStatsJoinTest.cpp index acc9816c7..82a6abf18 100644 --- a/axiom/optimizer/tests/UnknownStatsJoinTest.cpp +++ b/axiom/optimizer/tests/UnknownStatsJoinTest.cpp @@ -25,8 +25,14 @@ using namespace velox; // When a join-key NDV is missing the join cost is unknown, so the optimizer // falls back to the query's syntactic join order instead of a cost-based one. // 't' is large and 'u' is small; 'k' is the join key. -class UnknownStatsJoinTest : public test::QueryTestBase { +class UnknownStatsJoinTest : public test::QueryTestBase, + public ::testing::WithParamInterface { protected: + void SetUp() override { + useV2_ = GetParam(); + test::QueryTestBase::SetUp(); + } + velox::core::PlanNodePtr plan(const std::string& sql) { return toSingleNodePlan(parseSelect(sql, kTestConnectorId)); } @@ -43,7 +49,7 @@ class UnknownStatsJoinTest : public test::QueryTestBase { } }; -TEST_F(UnknownStatsJoinTest, singleJoin) { +TEST_P(UnknownStatsJoinTest, singleJoin) { testConnector_->addTable("t", ROW({"a", "k"}, BIGINT())) ->setStats(1'000'000, {{"k", {.numDistinct = 1'000'000}}}); testConnector_->addTable("u", ROW({"b", "k"}, BIGINT())) @@ -76,7 +82,7 @@ TEST_F(UnknownStatsJoinTest, singleJoin) { // cost-based ordering of an independent join elsewhere in the query. A // non-deterministic filter between (u JOIN t) and the join with 'v' keeps the // two joins in separate derived tables. -TEST_F(UnknownStatsJoinTest, twoJoins) { +TEST_P(UnknownStatsJoinTest, twoJoins) { const auto query = "SELECT count(*) " "FROM (SELECT u.k AS k FROM u JOIN t ON u.k = t.k WHERE rand() < 0.1) AS s " @@ -93,7 +99,7 @@ TEST_F(UnknownStatsJoinTest, twoJoins) { const std::string& innerBuild) { return matchScan(innerProbe) .hashJoinInner(matchScan(innerBuild)) - .filter() + .filterIf(!useV2_) .hashJoinInner(matchScan("v")) .aggregation() .build(); @@ -119,7 +125,7 @@ TEST_F(UnknownStatsJoinTest, twoJoins) { } // A base table with no statistics at all must fall back to syntactic join // order, not crash on the unknown cardinality. -TEST_F(UnknownStatsJoinTest, joinWithUnknownTableCardinality) { +TEST_P(UnknownStatsJoinTest, joinWithUnknownTableCardinality) { testConnector_->addTable("t", ROW({"a", "k"}, BIGINT())) ->setStats(1'000'000, {{"k", {.numDistinct = 1'000'000}}}); testConnector_->addTable("u", ROW({"b", "k"}, BIGINT())); @@ -140,7 +146,7 @@ TEST_F(UnknownStatsJoinTest, joinWithUnknownTableCardinality) { // Two large tables join only through 'v'; the fallback must hash-join through // it rather than cross-join the two. -TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoin) { +TEST_P(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoin) { addSharedJoinTableSchema(); const auto query = @@ -148,7 +154,7 @@ TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoin) { // Fallback on: hash-join 't' and 'u' through 'v'. optimizerOptions_.syntacticJoinOrder = false; - AXIOM_ASSERT_PLAN( + AXIOM_ASSERT_PLAN_V1( plan(query), matchScan("t") .hashJoinInner(matchScan("v")) @@ -168,7 +174,7 @@ TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoin) { } // An expression equi-key ('t.k + 0') behaves the same under the flag toggle. -TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoinExpressionKey) { +TEST_P(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoinExpressionKey) { addSharedJoinTableSchema(); const auto query = @@ -176,7 +182,7 @@ TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoinExpressionKey) { // Fallback on: hash-join through 'v' on the projected key. optimizerOptions_.syntacticJoinOrder = false; - AXIOM_ASSERT_PLAN( + AXIOM_ASSERT_PLAN_V1( plan(query), matchScan("t") .project() @@ -199,7 +205,7 @@ TEST_F(UnknownStatsJoinTest, sharedTableJoinAvoidsCrossJoinExpressionKey) { // Guard: 'w' has no equi-join to any table, so it still cross-joins even while // 't' and 'u' hash-join through the shared table 'v'. -TEST_F(UnknownStatsJoinTest, crossJoinWhenNoEquiPartner) { +TEST_P(UnknownStatsJoinTest, crossJoinWhenNoEquiPartner) { addSharedJoinTableSchema(); testConnector_->addTable("w", ROW({"c", "k"}, BIGINT())); @@ -209,7 +215,7 @@ TEST_F(UnknownStatsJoinTest, crossJoinWhenNoEquiPartner) { // Fallback on: 't'/'u' hash-join through 'v'; 'w' has no partner, so it // crosses. optimizerOptions_.syntacticJoinOrder = false; - AXIOM_ASSERT_PLAN( + AXIOM_ASSERT_PLAN_V1( plan(query), matchScan("t") .hashJoinInner(matchScan("v")) @@ -231,7 +237,7 @@ TEST_F(UnknownStatsJoinTest, crossJoinWhenNoEquiPartner) { } // The join sampler must tolerate an unknown build-side cardinality. -TEST_F(UnknownStatsJoinTest, sampledJoinWithUnknownCardinality) { +TEST_P(UnknownStatsJoinTest, sampledJoinWithUnknownCardinality) { optimizerOptions_.sampleJoins = true; testConnector_->addTable("t", ROW({"a", "k"}, BIGINT())) @@ -251,7 +257,7 @@ TEST_F(UnknownStatsJoinTest, sampledJoinWithUnknownCardinality) { // Enabling sampleJoins must not change the chosen plan when a side has // unknown cardinality. -TEST_F(UnknownStatsJoinTest, sampledJoinMatchesUnsampledOnUnknownCardinality) { +TEST_P(UnknownStatsJoinTest, sampledJoinMatchesUnsampledOnUnknownCardinality) { testConnector_->addTable("t", ROW({"a", "k"}, BIGINT())) ->setStats(1'000'000, {{"k", {.numDistinct = 1'000'000}}}); testConnector_->addTable("u", ROW({"b", "k"}, BIGINT())); @@ -275,5 +281,7 @@ TEST_F(UnknownStatsJoinTest, sampledJoinMatchesUnsampledOnUnknownCardinality) { AXIOM_ASSERT_PLAN(plan(altQuery), matchJoin("t", "u")); } +AXIOM_INSTANTIATE_V1_V2(UnknownStatsJoinTest); + } // namespace } // namespace facebook::axiom::optimizer