Skip to content
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.

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

Allow printing a plan outside of a transaction #890

Merged
merged 2 commits into from Jun 12, 2019

Conversation

5 participants
@dain
Copy link
Member

commented Jun 3, 2019

Clean up hack for generating plan for completion event with auto commit transactions.

@cla-bot cla-bot bot added the cla-signed label Jun 3, 2019

@dain dain requested a review from martint Jun 3, 2019

@@ -114,10 +121,27 @@ private StageExecutionPlan doPlan(SubPlan root, Session session, ImmutableList.B
dependencies.add(doPlan(childPlan, session, allSplitSources));
}

// extract TableInfo
Map<PlanNodeId, TableInfo> tables = new HashMap<>();
root.getFragment().getRoot().accept(

This comment has been minimized.

Copy link
@kokosing

kokosing Jun 3, 2019

Member

Maybe you could use io.prestosql.sql.planner.optimizations.PlanNodeSearcher#searchFrom(io.prestosql.sql.planner.plan.PlanNode, io.prestosql.sql.planner.iterative.Lookup)?

searchFrom( root.getFragment().getRoot())
  .where(TableScanNode.class::isInstanceOf)
  .findAll()
  .stream()
  .collect(toImmutableMap(...));
@findepi

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

@dain does Add partial field to SortNode fix prestodb/presto#12087?

@@ -883,7 +884,8 @@ private PlanBuilder sort(PlanBuilder subPlan, Optional<OrderBy> orderBy, List<Ex
new SortNode(
idAllocator.getNextId(),
subPlan.getRoot(),
new OrderingScheme(orderBySymbols.build(), orderings)));
new OrderingScheme(orderBySymbols.build(), orderings),
isDistributedSortEnabled(session)));

This comment has been minimized.

Copy link
@sopel39

sopel39 Jun 3, 2019

Member

Split between partial/non partial sort happens in io.prestosql.sql.planner.optimizations.AddExchanges.Rewriter#visitSort

@dain

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2019

@dain does Add partial field to SortNode fix prestodb/presto#12087?

My PR retains the existing behavior, which is "any sort node is considered partial of the flag is on". My goal is to eliminate session from here to prevent future uses of it in plan printing.

I can take a crack a moving this to AddExchanges if that is what you and @sopel39 want.

@dain dain force-pushed the dain:plan-printer branch 2 times, most recently from 5d20f71 to d7584a1 Jun 4, 2019

@sopel39

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

I can take a crack a moving this to AddExchanges if that is what you and @sopel39 want.

@dain it should be extremely easy. Partial exchange is created here: io/prestosql/sql/planner/optimizations/AddExchanges.java:439

@dain dain force-pushed the dain:plan-printer branch from d7584a1 to fd4cadd Jun 6, 2019

@dain

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2019

@sopel39 I updated to set partial in the right place.

@martint this is ready to review

@dain dain force-pushed the dain:plan-printer branch from fd4cadd to 50141f8 Jun 7, 2019

@dain dain force-pushed the dain:plan-printer branch from 50141f8 to 19c6947 Jun 11, 2019

@dain dain requested a review from martint Jun 12, 2019

@dain dain merged commit 8c447ea into prestosql:master Jun 12, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
verification/cla-signed
Details

@dain dain deleted the dain:plan-printer branch Jun 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.