Skip to content

Commit

Permalink
Fix planning failure for large IN lists
Browse files Browse the repository at this point in the history
  • Loading branch information
electrum committed Sep 25, 2015
1 parent c2705d7 commit 5e3a182
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
1 change: 1 addition & 0 deletions presto-docs/src/main/sphinx/release/release-0.120.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ General Changes

* Fix regression that causes task scheduler to not retry requests in some cases.
* Throttle task info refresher on errors.
* Fix planning failure that prevented the use of large ``IN`` lists.
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;

import java.util.ArrayDeque;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Queue;

import static com.facebook.presto.sql.tree.BooleanLiteral.FALSE_LITERAL;
import static com.facebook.presto.sql.tree.BooleanLiteral.TRUE_LITERAL;
Expand All @@ -46,6 +47,7 @@
import static com.facebook.presto.util.ImmutableCollectors.toImmutableList;
import static com.google.common.base.Predicates.not;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Lists.newArrayList;
import static java.util.Objects.requireNonNull;

public final class ExpressionUtils
Expand Down Expand Up @@ -104,14 +106,12 @@ public static Expression binaryExpression(LogicalBinaryExpression.Type type, Ite
requireNonNull(expressions, "expressions is null");
Preconditions.checkArgument(!Iterables.isEmpty(expressions), "expressions is empty");

Iterator<Expression> iterator = expressions.iterator();

Expression result = iterator.next();
while (iterator.hasNext()) {
result = new LogicalBinaryExpression(type, result, iterator.next());
// build balanced tree for efficient recursive processing
Queue<Expression> queue = new ArrayDeque<>(newArrayList(expressions));
while (queue.size() > 1) {
queue.add(new LogicalBinaryExpression(type, queue.remove(), queue.remove()));
}

return result;
return queue.remove();
}

public static Expression combineConjuncts(Expression... expressions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.facebook.presto.testing.QueryRunner;
import com.facebook.presto.type.TypeRegistry;
import com.facebook.presto.util.DateTimeZoneIndex;
import com.google.common.base.Joiner;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -73,6 +74,7 @@
import static io.airlift.tpch.TpchTable.ORDERS;
import static io.airlift.tpch.TpchTable.tableNameGetter;
import static java.lang.String.format;
import static java.util.stream.IntStream.range;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
Expand Down Expand Up @@ -3135,6 +3137,9 @@ public void testIn()
assertQuery("SELECT orderkey FROM orders WHERE orderkey IN (1.5, 2.3)", "SELECT orderkey FROM orders LIMIT 0"); // H2 incorrectly matches rows
assertQuery("SELECT orderkey FROM orders WHERE orderkey IN (1, 2.0, 3)");
assertQuery("SELECT orderkey FROM orders WHERE totalprice IN (1, 2, 3)");

String list = Joiner.on(',').join(range(0, 20_000).boxed().iterator());
assertQuery("SELECT orderkey FROM orders WHERE orderkey IN (" + list + ")");
}

@Test
Expand Down

0 comments on commit 5e3a182

Please sign in to comment.