Extend fast inequality join #8614

anusudarsan · 2017-07-27T13:26:36Z

This extends functionality added in #7097, for #6922.

Internal review - Teradata#630

The PR extends the functionality to speed up query with range predicates eg: benchmarkRangePredicateJoin . But I added benchmark tests for other queries which were already addressed by the optimization. So you can see the comparison below with and without this optimization.

								(buckets)  (fastInequalityJoins)     (master)	            (PR branch)
BenchmarkInequalityJoin.benchmarkJoin 				100		true		222.267 ±  36.490  ms/op   234.191 ±  33.999  ms/op
BenchmarkInequalityJoin.benchmarkJoin 				100		false	       2409.789 ± 193.371  ms/op  2360.016 ± 189.837  ms/op
BenchmarkInequalityJoin. benchmarkJoinWithArithmeticInPredicate	100		true		279.125 ±  23.798  ms/op   280.396 ±  17.991  ms/op
BenchmarkInequalityJoin. benchmarkJoinWithArithmeticInPredicate	100		false	       2375.963 ±  78.662  ms/op  2376.180 ± 125.109  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate	100		true		193.858 ±  12.845  ms/op   216.786 ±  12.600  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate	100		false	       2288.445 ±  55.931  ms/op  2408.483 ±  97.140  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin		100		true	      2435.688 ± 143.372  ms/op	   247.428 ±  11.549  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin		100		false	      2433.708 ±  64.086  ms/op   2487.442 ±  60.085  ms/op

Complete Benchmarking results

Benchmark                                                     (buckets)  (fastInequalityJoins)  (filterOutCoefficient)  Mode  Cnt     Score     Error  Units
BenchmarkInequalityJoin.benchmarkJoin                               100                   true                      10  avgt   30   234.191 ±  33.999  ms/op
BenchmarkInequalityJoin.benchmarkJoin                               100                  false                      10  avgt   30  2360.016 ± 189.837  ms/op
BenchmarkInequalityJoin.benchmarkJoin                              1000                   true                      10  avgt   30   187.426 ±  24.792  ms/op
BenchmarkInequalityJoin.benchmarkJoin                              1000                  false                      10  avgt   30   414.487 ±  27.297  ms/op
BenchmarkInequalityJoin.benchmarkJoin                             10000                   true                      10  avgt   30   198.977 ±  35.756  ms/op
BenchmarkInequalityJoin.benchmarkJoin                             10000                  false                      10  avgt   30   239.980 ±  18.026  ms/op
BenchmarkInequalityJoin.benchmarkJoin                             60000                   true                      10  avgt   30   173.009 ±   6.956  ms/op
BenchmarkInequalityJoin.benchmarkJoin                             60000                  false                      10  avgt   30   181.165 ±   8.649  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate        100                   true                      10  avgt   30   280.396 ±  17.991  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate        100                  false                      10  avgt   30  2376.180 ± 125.109  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate       1000                   true                      10  avgt   30   210.539 ±  11.744  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate       1000                  false                      10  avgt   30   471.721 ±  45.251  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate      10000                   true                      10  avgt   30   203.669 ±   9.373  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate      10000                  false                      10  avgt   30   259.281 ±  13.036  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate      60000                   true                      10  avgt   30   203.048 ±  10.953  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithArithmeticInPredicate      60000                  false                      10  avgt   30   199.349 ±   9.362  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate          100                   true                      10  avgt   30   216.786 ±  12.600  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate          100                  false                      10  avgt   30  2408.483 ±  97.140  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate         1000                   true                      10  avgt   30   195.763 ±  13.215  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate         1000                  false                      10  avgt   30   580.025 ± 120.265  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate        10000                   true                      10  avgt   30   226.885 ±  24.685  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate        10000                  false                      10  avgt   30   274.404 ±  18.313  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate        60000                   true                      10  avgt   30   197.643 ±  11.469  ms/op
BenchmarkInequalityJoin.benchmarkJoinWithFunctionPredicate        60000                  false                      10  avgt   30   210.390 ±  15.268  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin                 100                   true                      10  avgt   30   247.428 ±  11.549  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin                 100                  false                      10  avgt   30  2487.442 ±  60.085  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin                1000                   true                      10  avgt   30   240.810 ±  14.584  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin                1000                  false                      10  avgt   30   527.124 ±  47.483  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin               10000                   true                      10  avgt   30   226.683 ±  11.559  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin               10000                  false                      10  avgt   30   270.130 ±  13.167  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin               60000                   true                      10  avgt   30   226.237 ±   8.305  ms/op
BenchmarkInequalityJoin.benchmarkRangePredicateJoin               60000                  false                      10  avgt   30   218.149 ±   9.184  ms/op

@losipiuk

losipiuk · 2017-08-10T14:26:23Z

@kokosing, @sopel39 , @anusudarsan I cleaned this up as the whole feature from the start felt to messy to me.
The refactoring commits should probably by squashed together (please decide). I leave those separated for sake of review.

Please give it (hopefully) last read.

anusudarsan

@losipiuk LGTM mod comment. Also, I fixed a checkstyle error.

anusudarsan · 2017-08-17T07:34:45Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

        // check if filtering function to startingPosition
-        if (applyLessThanFunction(startingPosition, probePosition, allProbeChannelsPage)) {
+        if (applyLessThanFunction(filterFunction, startingPosition, probePosition, allProbeChannelsPage)) {


Should we rename applyLessThanFunction -> applySearchFunction as a part of the refactoring commit 9e5b4b38040a59120bd3de6ac67c832403efb97e ?

I added renaming commit.

losipiuk · 2017-08-18T12:46:28Z

@sopel39 last pass?

findepi

One important comment about correctness, a bit of refactor and wording. Feel free to ignore anything you find a nit-picking.

findepi · 2017-08-22T07:45:40Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

@@ -44,13 +44,14 @@
 * <p>


while cleanup up this javadoc, first line is:

This class assumes that lessThanFunction is a superset of the whole filtering

This is remotely related to truth. Before other changes in this PR, no ANDs nor ORs are supported.
After other changes, only specific ANDs and ORs are supported (those that compare to same build side symbol).

Further, in the following

by passing any of the filterFunction_i to the SortedPositionLinks. We could not

s/We could not/We cannot/

I simplified the Javadoc very much.

findepi · 2017-08-22T07:50:39Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

- * That allows us to define an order of the elements in positionLinks (this defining which
- * element is smaller) using {@code g(...)} function and to perform a binary search using
- * {@code f(probePosition)} value.
+ * and buildSymbolRef is a symbol reference on the build side of join.


s/buildSymbolRef/{@code buildSymbolRef}

findepi · 2017-08-22T07:51:18Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

+ * and buildSymbolRef is a symbol reference on the build side of join.
+ *
+ * That allows us to define an order of the elements in positionLinks by sorting them by
+ * {@code buildSymbolRef} and then perform a binary search using {@code f(probePosition)}.


s/{@code f(probePosition)}/{@code f(probeColumn1, probeColumn2, ..., probeColumnN)}

note about binary search is a bit misleading, we do binary search only for >, >= cases

I will think about the binary search case

findepi · 2017-08-22T07:53:19Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

- * A.a < B.x
+ * A.a < f(B.x, B.y, B.z)
+ * <p>
+ * where a is the build side symbol reference and x,y,z are probe


s/a/{@code a}, same for x,y,z

findepi · 2017-08-22T08:01:20Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

@@ -32,7 +32,10 @@
 /**
 * Currently this class handles only simple expressions like:
 * <p>
- * A.a < B.x
+ * A.a < f(B.x, B.y, B.z)


In SortedPositionLinks you said about >, <, <=, >=. Here, < is only for terseness or this class indeed supports only <?

findepi · 2017-08-22T12:15:12Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

    }

    @Override
    public int start(int startingPosition, int probePosition, Page allProbeChannelsPage)
    {
+        for (JoinFilterFunction searchFunction : searchFunctions) {
+            startingPosition = findStartPositionForFunction(searchFunction, startingPosition, probePosition, allProbeChannelsPage);


pls do not overwrite input param startingPosition

findepi · 2017-08-22T12:20:52Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+            Optional<SortExpressionContext> rightProcessed = process(binaryExpression.getRight());
+
+            if (!leftProcessed.isPresent() || !rightProcessed.isPresent() || !leftProcessed.get().getSortExpression().equals(rightProcessed.get().getSortExpression())) {
+                return Optional.empty();


Return left (ie any) and put TODO to make it cost-based decision.
If there are two < conditions on two different build symbols, better to have sorted position links on one of them than on none.

findepi · 2017-08-22T12:22:04Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+        private SortExpressionContext merge(SortExpressionContext left, SortExpressionContext right)
+        {
+            checkArgument(left.getSortExpression().equals(right.getSortExpression()));
+            ImmutableSet.Builder<Expression> searchExpressions = ImmutableSet.builder();


Why not a list? Expression de-duplication is something optimizer should be doing, a list should suffice.

findepi · 2017-08-22T12:24:34Z

presto-main/src/test/java/com/facebook/presto/sql/planner/TestSortExpressionExtractor.java

    }

-    private static void assertGetSortExpression(Expression expression)
+    private void assertGetSortExpression(Expression expression)


why no longer static?

findepi · 2017-08-22T12:24:59Z

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java

@@ -2338,6 +2338,22 @@ public void testJoinWithGreaterThanInJoinClause()
    }

    @Test
+    public void testJoinWithRangePredicatesinJoinClause()
+    {
+        assertQuery("SELECT COUNT(*)\n" +


replace \n with in the queries

findepi · 2017-08-22T16:57:03Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

@@ -131,11 +131,11 @@ protected Boolean visitSymbolReference(SymbolReference symbolReference, Void con
        }
    }

-    public static class SortExpression
+    public static class RowSortExpressionContext


is there a point in renaming & extracting the class you remove 2 commits later?

Discussed. Let's leave it.

losipiuk · 2017-08-24T08:36:07Z

@anusudarsan I am addressing the comments

findepi · 2017-08-24T14:00:00Z

presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/PlanPrinter.java

@@ -554,7 +554,7 @@ public Void visitJoin(JoinNode node, Integer indent)
                        formatOutputs(node.getOutputSymbols()));
            }

-            node.getSortExpression().ifPresent(expression -> print(indent + 2, "SortExpression[%s]", expression));
+            node.getSortExpressionContext().ifPresent(context -> print(indent + 2, "SortExpression[%s]", context.getSortExpression()));


Interesting what does it do in plan printer. Unconditionally to the flag enabling inequi join. (no change requested)

findepi · 2017-08-24T14:02:10Z

presto-main/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java

@@ -1591,14 +1591,17 @@ private LookupSourceFactory createLookupSourceFactory(
                            context.getTypes(),
                            context.getSession()));

-            Optional<Integer> sortChannel = node.getSortExpressionContext()
+            Optional<SortExpressionContext> sortExpressionContext = node.getSortExpressionContext();


findepi · 2017-08-24T14:03:37Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

        checkState(!searchFunctions.isEmpty(), "Using sortedPositionLinks with no search functions");
-        this.searchFunctions = requireNonNull(searchFunctions, "searchFunctions is null");
+        this.searchFunctions = searchFunctions;


Consider converting it to array here.

findepi · 2017-08-24T14:04:27Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

@@ -192,53 +193,54 @@ public int next(int position, int probePosition, Page allProbeChannelsPage)
    @Override
    public int start(int startingPosition, int probePosition, Page allProbeChannelsPage)
    {
+        int[] links = sortedPositionLinks[startingPosition];
+        if (links == null) {
+            return -1;


Isn't links==null the case of no collisions?

In original code, we had the following here:

if (applyLessThanFunction(startingPosition, probePosition, allProbeChannelsPage)) { return startingPosition; }

and this seemingly could return startingPosition even if sortedPositionLinks[startingPosition]==null

findepi · 2017-08-24T14:08:40Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

-        int left = 0;
-        int right = sortedPositionLinks[startingPosition].length - 1;
+        int left = startOffset;
+        int right = links.length - 1;


You can safely inline left & right

findepi · 2017-08-24T14:10:09Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

        if (offset < 0) {
            return -1;
        }
-        if (!applySearchFunction(searchFunction, startingPosition, offset, probePosition, allProbeChannelsPage)) {
+        if (!applySearchFunction(searchFunction, links, offset, probePosition, allProbeChannelsPage)) {


Now i dont understand what this if does here. Looks like it's redundant and lowerBound could cover it. (no change required, but you can think about it..)

findepi · 2017-08-24T14:12:30Z

presto-main/src/main/java/com/facebook/presto/sql/planner/LocalExecutionPlanner.java

+            List<JoinFilterFunctionFactory> searchFunctionFactories = sortExpressionContext
+                    .map(SortExpressionContext::getSearchExpressions)
+                    .map(searchExpressions -> searchExpressions
+                            .stream()


retain .stream() on prev line

findepi · 2017-08-24T14:15:29Z

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java

@@ -5276,7 +5284,7 @@ public void testWithHiding()
                        "SELECT * FROM b",
                "SELECT 2");
        assertQueryFails(
-                        "WITH a AS (VALUES 1), " +
+                "WITH a AS (VALUES 1), " +


findepi · 2017-08-28T11:32:37Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

+        for (JoinFilterFunction searchFunction : searchFunctions) {
+            if (!applySearchFunction(searchFunction, startingPosition, probePosition, allProbeChannelsPage)) {
+                allSearchFunctionsMatchedForStartingPosition = false;
+                break;


simply return false

findepi · 2017-08-28T11:34:54Z

presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java

+                        "(VALUES (1,1),(2,1)) t1(a,b), " +
+                        "(VALUES (1,1),(1,2),(2,1)) t2(x,y) " +
+                        "WHERE a=x and b>=y",
+                "VALUES (1,1,1,1), (2,1,2,1)");


Add a test in TestPositionLinks too

findepi · 2017-08-28T11:37:15Z

presto-main/src/main/java/com/facebook/presto/operator/SortedPositionLinks.java

@@ -208,6 +211,18 @@ public int start(int startingPosition, int probePosition, Page allProbeChannelsP
        return links[currentStartOffset];
    }

+    private boolean applyAllSearchFunctions(int startingPosition, int probePosition, Page allProbeChannelsPage)


Shan't this new cool method be used in next() too?

findepi · 2017-08-29T10:42:22Z

presto-main/src/test/java/com/facebook/presto/operator/TestPositionLinks.java

@@ -61,7 +61,7 @@ public void testArrayPositionLinks()
    public void testSortedPositionLinks()
    {
        JoinFilterFunction filterFunction = (leftAddress, rightPosition, rightPage) ->
-                BIGINT.getLong(rightPage.getBlock(0), leftAddress) > 4;
+                BIGINT.getLong(TEST_PAGE.getBlock(0), leftAddress) > 4;


cmt msg typo "embended"

losipiuk · 2017-08-29T11:42:13Z

Squashed fixups.

findepi · 2017-08-29T11:50:23Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionContext.java

+
+    public SortExpressionContext(Expression sortExpression)
+    {
+        this.sortExpression = requireNonNull(sortExpression, "sortExpression can not be null");


"can not" is not valid, see #8770

findepi · 2017-08-29T12:00:10Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+            Optional<SortExpressionContext> rightProcessed = process(binaryExpression.getRight());
+
+            if (!leftProcessed.isPresent() || !rightProcessed.isPresent() || !leftProcessed.get().getSortExpression().equals(rightProcessed.get().getSortExpression())) {
+                return Optional.empty();


losipiuk · 2017-09-08T11:58:57Z

@findepi. Would you like to take a look in some spare time? ;)

findepi

Only minor comments

findepi · 2017-09-10T19:17:53Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+                .collect(groupingBy(SortExpressionContext::getSortExpression, reducing(SortExpressionExtractor::merge)))
+                .values()
+                .stream()
+                .map(Optional::get)


This looks suspicious: call to Optional.get without .ifPresent. You can avoid having those misleading Optional-s, if you replace .collect(groupingBy(...)) with .collect(toMap(SortExpressionContext::getSortExpression, c->c, SortExpressionExtractor::merge).

findepi · 2017-09-10T19:18:48Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.groupingBy;


cmt msg "Consider different sort expressions in SortExpressionExtractor" -- it is not quite clear. Maybe

"Extract sort expressions from complex join filters" ?

findepi · 2017-09-10T19:19:30Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+                .map(Optional::get)
+                .collect(toImmutableList());
+
+        // For now heuristically pick sort expression which has most search expression assigned to it.


s/search expression/&s

findepi · 2017-09-10T19:20:18Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+
+        // For now heuristically pick sort expression which has most search expression assigned to it.
+        // TODO: make it cost based decision based on symbol statistics
+        return sortExpressionCandidates


findepi · 2017-09-10T19:22:05Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+        // TODO: make it cost based decision based on symbol statistics
+        return sortExpressionCandidates
+                .stream()
+                .sorted(comparing(context -> -context.getSearchExpressions().size()))


Consider using .reversed() instead of -

I tried that before. As well as Comparator.reverseOrder. But then I have to add explicit cast of context to SortExpressionContext.
Like this:

.sorted(comparing(context -> ((SortExpressionContext) context).getSearchExpressions().size()).reversed())

I prefer - in that case unless you can help me get rid of cast.

Alas. However, rather than cast, you should rather s/context ->/(SortExpressionContext context) ->.

You can also -1 * instead of - to make it more visible. Or leave as is, not a big deal.

losipiuk · 2017-09-11T07:03:08Z

Addressed comments. One question.

findepi · 2017-09-11T07:47:18Z

presto-main/src/main/java/com/facebook/presto/sql/planner/SortExpressionExtractor.java

+        // TODO: make it cost based decision based on symbol statistics
+        return sortExpressionCandidates
+                .stream()
+                .sorted(comparing(context -> -context.getSearchExpressions().size()))


Alas. However, rather than cast, you should rather s/context ->/(SortExpressionContext context) ->.

You can also -1 * instead of - to make it more visible. Or leave as is, not a big deal.

Use Row prefix to point fact that class operates in channels domain.

Add SortExpressionContext which captures logical sort expression.

…ntext

Pass explicit searchExpression when doing inequality filtering for join. Previously whole filterFunction was assummed to be the search expression. With explici searchExpression we can capture more cases when we want to use subset of filter function conjunts (possibly transformed) as search function.

The sorted position links is searched for each of the expression in the range predicate. Thus this optimization works only for predicates with AND (conjuncts). The iteration over the position links is stopped as soon as any of the filter expression evaluates to false.

TestPositionLinks used `rightPage` to simulate access to build-side data. Changing code to use TEST_PAGE which more accuratelly simulates implementation of behaviour of standard implementation of JoinFilterFunction which have build-side data embeded.

facebook-github-bot added the CLA Signed label Jul 27, 2017

anusudarsan mentioned this pull request Jul 27, 2017

Extend fast inequality join Teradata/presto#630

Closed

losipiuk force-pushed the extend-fast-inequality-join branch 2 times, most recently from 758a8ee to c292c1e Compare August 10, 2017 14:23

losipiuk requested review from kokosing and sopel39 August 10, 2017 14:24

losipiuk force-pushed the extend-fast-inequality-join branch from c292c1e to 0750369 Compare August 10, 2017 14:28

anusudarsan force-pushed the extend-fast-inequality-join branch from 0750369 to 20cc040 Compare August 17, 2017 07:50

anusudarsan commented Aug 17, 2017

View reviewed changes

losipiuk force-pushed the extend-fast-inequality-join branch 3 times, most recently from 8c10e51 to 4e689f5 Compare August 18, 2017 12:46

findepi suggested changes Aug 22, 2017

View reviewed changes

findepi reviewed Aug 22, 2017

View reviewed changes

losipiuk force-pushed the extend-fast-inequality-join branch from 4e689f5 to 4229b05 Compare August 24, 2017 13:55

findepi suggested changes Aug 24, 2017

View reviewed changes

findepi reviewed Aug 28, 2017

View reviewed changes

findepi approved these changes Aug 29, 2017

View reviewed changes

losipiuk force-pushed the extend-fast-inequality-join branch from 4d9bcbf to d32d2ed Compare August 29, 2017 11:41

findepi approved these changes Aug 29, 2017

View reviewed changes

losipiuk force-pushed the extend-fast-inequality-join branch 3 times, most recently from 1563e88 to 7883b37 Compare September 8, 2017 11:57

losipiuk force-pushed the extend-fast-inequality-join branch from 7883b37 to 1f0e036 Compare September 8, 2017 14:18

findepi approved these changes Sep 10, 2017

View reviewed changes

findepi approved these changes Sep 11, 2017

View reviewed changes

losipiuk force-pushed the extend-fast-inequality-join branch from f4deff6 to f4e0b62 Compare September 11, 2017 08:09

losipiuk and others added 10 commits September 11, 2017 10:13

Use proper session in MemoryLocalQueryRunner

e07fffc

Remove unused field from inequality join optimization

0d34272

Simplify javadoc for SortedPositionLinks and SortExpressionExtractor

6db0b36

Update JoinFilterCacheKey equals and hashCode

2a10ce1

Add tests for non-equi join condition optimization

db2e844

Rename SortExpression to RowSortExpressionContext

16dab33

Use Row prefix to point fact that class operates in channels domain.

Move RowSortExpressionContext to top level

b1911bd

Add SortExpressionContext

a50fb91

Add SortExpressionContext which captures logical sort expression.

Explicitly pass sortChannel where needed and drop RowSortExpressionCo…

adb2cc6

…ntext

losipiuk force-pushed the extend-fast-inequality-join branch from f4e0b62 to 318332a Compare September 11, 2017 08:13

losipiuk and others added 10 commits September 11, 2017 10:13

Rename lessThanFunction to searchFunction in SortedPositionLinks

8a2a194

Refactor unit tests to use expression utility

081f6c4

Remove extranous Javadoc

6a64e5d

Add comment to TestPositionLinks

26b7c9a

Replace anonymous classess with lambdas

e567484

Extend inequality testcases in TestPositionLinks

34c51de

Extract sort expressions from complex join filters

0f74747

Allow some conjuncts to be nondeterministic in SortExpressionExtractor

318332a

losipiuk merged commit 168e3ee into prestodb:master Sep 11, 2017

mbasmanova mentioned this pull request Jun 10, 2024

[WIP] Merge NestedLoopJoin Build vectors facebookincubator/velox#10048

Closed

aditi-pandit mentioned this pull request Jun 14, 2024

[Native] - Correlated query returning incorrect results (Streaming agg assumes nested join produces pre-grouped results) #22585

Closed

aditi-pandit mentioned this pull request Jun 21, 2024

Equivalent of Presto Lookup Join in Velox facebookincubator/velox#10284

Closed

		import static java.util.Objects.requireNonNull;
		import static java.util.stream.Collectors.groupingBy;

Extend fast inequality join #8614

Extend fast inequality join #8614

Conversation

anusudarsan commented Jul 27, 2017 • edited Loading

losipiuk commented Aug 10, 2017

anusudarsan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk commented Aug 18, 2017

findepi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk commented Aug 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk commented Aug 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk commented Sep 8, 2017

findepi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

losipiuk commented Sep 11, 2017

Choose a reason for hiding this comment

anusudarsan commented Jul 27, 2017 •

edited

Loading