Skip to content

Fix optimizer performance regression for large IN #22664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ZacBlanco merged 3 commits into from
Jun 4, 2024
Merged

Fix optimizer performance regression for large IN #22664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2f2b19
Add histograms for optimizer cost calculation
ZacBlanco Feb 28, 2024
371b40c
Fix optimizer performance regression for large IN
ZacBlanco May 3, 2024
75c1c07
Gate histogram calcs by session property
ZacBlanco May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 56 additions & 6 deletions ...o-benchmarks/src/test/java/com/facebook/presto/sql/planner/AbstractCostBasedPlanTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.facebook.presto.sql.planner;

import com.facebook.presto.Session;
import com.facebook.presto.spi.ConnectorTableHandle;
import com.facebook.presto.spi.plan.AggregationNode;
import com.facebook.presto.spi.plan.JoinDistributionType;
Expand All @@ -37,6 +38,7 @@
import java.nio.file.Paths;
import java.util.stream.Stream;

import static com.facebook.presto.SystemSessionProperties.OPTIMIZER_USE_HISTOGRAMS;
import static com.facebook.presto.spi.plan.JoinDistributionType.REPLICATED;
import static com.facebook.presto.spi.plan.JoinType.INNER;
import static com.facebook.presto.sql.Optimizer.PlanStage.OPTIMIZED_AND_VALIDATED;
Expand Down Expand Up @@ -76,11 +78,42 @@ public void test(String queryResourcePath)
assertEquals(generateQueryPlan(read(queryResourcePath)), read(getQueryPlanResourcePath(queryResourcePath)));
}

@Test(dataProvider = "getQueriesDataProvider")
public void histogramsPlansMatch(String queryResourcePath)
{
String sql = read(queryResourcePath);
Session histogramSession = Session.builder(getQueryRunner().getDefaultSession())
.setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "true")
.build();
Session noHistogramSession = Session.builder(getQueryRunner().getDefaultSession())
.setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "false")
.build();
String regularPlan = generateQueryPlan(sql, noHistogramSession);
String histogramPlan = generateQueryPlan(sql, histogramSession);
if (!regularPlan.equals(histogramPlan)) {
assertEquals(histogramPlan, read(getHistogramPlanResourcePath(getQueryPlanResourcePath(queryResourcePath))));
}
}

private String getQueryPlanResourcePath(String queryResourcePath)
{
return queryResourcePath.replaceAll("\\.sql$", ".plan.txt");
}

private String getHistogramPlanResourcePath(String regularPlanResourcePath)
{
Path root = Paths.get(regularPlanResourcePath);
return root.getParent().resolve("histogram/" + root.getFileName()).toString();
}

private Path getResourceWritePath(String queryResourcePath)
{
return Paths.get(
getSourcePath().toString(),
"src/test/resources",
getQueryPlanResourcePath(queryResourcePath));
}

public void generate()
throws Exception
{
Expand All @@ -90,12 +123,24 @@ public void generate()
.parallel()
.forEach(queryResourcePath -> {
try {
Path queryPlanWritePath = Paths.get(
getSourcePath().toString(),
"src/test/resources",
getQueryPlanResourcePath(queryResourcePath));
Path queryPlanWritePath = getResourceWritePath(queryResourcePath);
createParentDirs(queryPlanWritePath.toFile());
write(generateQueryPlan(read(queryResourcePath)).getBytes(UTF_8), queryPlanWritePath.toFile());
Session histogramSession = Session.builder(getQueryRunner().getDefaultSession())
.setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "true")
.build();
Session noHistogramSession = Session.builder(getQueryRunner().getDefaultSession())
.setSystemProperty(OPTIMIZER_USE_HISTOGRAMS, "false")
.build();
String sql = read(queryResourcePath);
String regularPlan = generateQueryPlan(sql, noHistogramSession);
String histogramPlan = generateQueryPlan(sql, histogramSession);
write(regularPlan.getBytes(UTF_8), queryPlanWritePath.toFile());
// write out the histogram plan if it differs
if (!regularPlan.equals(histogramPlan)) {
Path histogramPlanWritePath = getResourceWritePath(getHistogramPlanResourcePath(queryResourcePath));
createParentDirs(histogramPlanWritePath.toFile());
write(histogramPlan.getBytes(UTF_8), histogramPlanWritePath.toFile());
}
System.out.println("Generated expected plan for query: " + queryResourcePath);
}
catch (IOException e) {
Expand All @@ -119,11 +164,16 @@ private static String read(String resource)
}

private String generateQueryPlan(String query)
{
return generateQueryPlan(query, getQueryRunner().getDefaultSession());
}

private String generateQueryPlan(String query, Session session)
{
String sql = query.replaceAll("\\s+;\\s+$", "")
.replace("${database}.${schema}.", "")
.replace("\"${database}\".\"${schema}\".\"${prefix}", "\"");
Plan plan = plan(sql, OPTIMIZED_AND_VALIDATED, false);
Plan plan = plan(session, sql, OPTIMIZED_AND_VALIDATED, false);

JoinOrderPrinter joinOrderPrinter = new JoinOrderPrinter();
plan.getRoot().accept(joinOrderPrinter, 0);
Expand Down
38 changes: 38 additions & 0 deletions presto-benchto-benchmarks/src/test/resources/sql/presto/tpcds/histogram/q85.plan.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
local exchange (GATHER, SINGLE, [])
remote exchange (GATHER, SINGLE, [])
final aggregation over (r_reason_desc)
local exchange (GATHER, SINGLE, [])
remote exchange (REPARTITION, HASH, [r_reason_desc])
partial aggregation over (r_reason_desc)
join (INNER, REPLICATED):
join (INNER, REPLICATED):
join (INNER, PARTITIONED):
remote exchange (REPARTITION, HASH, [cd_demo_sk, cd_education_status, cd_marital_status])
scan customer_demographics
local exchange (GATHER, SINGLE, [])
remote exchange (REPARTITION, HASH, [cd_education_status_3, cd_marital_status_2, wr_refunded_cdemo_sk])
join (INNER, PARTITIONED):
remote exchange (REPARTITION, HASH, [wr_refunded_addr_sk])
join (INNER, PARTITIONED):
remote exchange (REPARTITION, HASH, [ws_item_sk, ws_order_number])
join (INNER, REPLICATED):
scan web_sales
local exchange (GATHER, SINGLE, [])
remote exchange (REPLICATE, BROADCAST, [])
scan date_dim
local exchange (GATHER, SINGLE, [])
remote exchange (REPARTITION, HASH, [wr_item_sk, wr_order_number])
join (INNER, REPLICATED):
scan web_returns
local exchange (GATHER, SINGLE, [])
remote exchange (REPLICATE, BROADCAST, [])
scan customer_demographics
local exchange (GATHER, SINGLE, [])
remote exchange (REPARTITION, HASH, [ca_address_sk])
scan customer_address
local exchange (GATHER, SINGLE, [])
remote exchange (REPLICATE, BROADCAST, [])
scan web_page
local exchange (GATHER, SINGLE, [])
remote exchange (REPLICATE, BROADCAST, [])
scan reason
Loading