Skip to content

Commit

Permalink
Do not derive all grouping sets when traversing tree
Browse files Browse the repository at this point in the history
The only node that has visitable expressions is SimpleGroupBy. There's
no point in synthesizing the grouping sets from nodes like Cube or Rollup
just for the purpose of visiting the resulting expressions.

One effect of doing so is that query creation can fail due to OOMs
when allocating too many objects.
  • Loading branch information
martint committed Apr 24, 2018
1 parent a1d37b3 commit 480d0f2
Showing 1 changed file with 22 additions and 7 deletions.
Expand Up @@ -13,8 +13,6 @@
*/
package com.facebook.presto.sql.tree;

import java.util.Set;

public abstract class DefaultTraversalVisitor<R, C>
extends AstVisitor<R, C>
{
Expand Down Expand Up @@ -484,13 +482,30 @@ protected R visitGroupBy(GroupBy node, C context)
}

@Override
protected R visitGroupingElement(GroupingElement node, C context)
protected R visitCube(Cube node, C context)
{
return null;
}

@Override
protected R visitRollup(Rollup node, C context)
{
return null;
}

@Override
protected R visitSimpleGroupBy(SimpleGroupBy node, C context)
{
for (Set<Expression> expressions : node.enumerateGroupingSets()) {
for (Expression expression : expressions) {
process(expression, context);
}
for (Expression expression : node.getColumnExpressions()) {
process(expression, context);
}

return null;
}

@Override
protected R visitGroupingSets(GroupingSets node, C context)
{
return null;
}

Expand Down

0 comments on commit 480d0f2

Please sign in to comment.