You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2022/03/21 11:34:24 UTC
[ignite] branch sql-calcite updated: IGNITE-16693 Merge JOIN nulls processing - Fixes #9896.
This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch sql-calcite
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/sql-calcite by this push:
new 8e52955 IGNITE-16693 Merge JOIN nulls processing - Fixes #9896.
8e52955 is described below
commit 8e52955e7abafcefcf4a26af439991bf1019c1eb
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Mon Mar 21 14:28:35 2022 +0300
IGNITE-16693 Merge JOIN nulls processing - Fixes #9896.
Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
---
.../calcite/exec/exp/ExpressionFactoryImpl.java | 61 +++++++++--------
.../calcite/exec/rel/MergeJoinExecutionTest.java | 59 ++++++++++++++++-
.../calcite/integration/JoinIntegrationTest.java | 77 ++++++++++++++++++++++
3 files changed, 169 insertions(+), 28 deletions(-)
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
index b1ce953..579ca6c 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
@@ -30,7 +30,6 @@ import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Ordering;
import com.google.common.primitives.Primitives;
import org.apache.calcite.DataContext;
import org.apache.calcite.adapter.enumerable.EnumUtils;
@@ -164,40 +163,48 @@ public class ExpressionFactoryImpl<Row> implements ExpressionFactory<Row> {
throw new IllegalArgumentException("Both inputs should be non-empty and have the same size: left="
+ (left != null ? left.size() : "null") + ", right=" + (right != null ? right.size() : "null"));
- List<Comparator<Row>> comparators = new ArrayList<>();
+ // Check that collations is correct.
+ for (int i = 0; i < left.size(); i++) {
+ if (left.get(i).nullDirection.nullComparison != right.get(i).nullDirection.nullComparison)
+ throw new IllegalArgumentException("Can't be compared: left=" + left.get(i) + ", right=" + right.get(i));
- for (int i = 0; i < left.size(); i++)
- comparators.add(comparator(left.get(i), right.get(i)));
+ if (left.get(i).direction != right.get(i).direction)
+ throw new IllegalArgumentException("Can't be compared: left=" + left.get(i) + ", right=" + right.get(i));
+ }
- return Ordering.compound(comparators);
- }
+ return new Comparator<Row>() {
+ @Override public int compare(Row o1, Row o2) {
+ boolean hasNulls = false;
+ RowHandler<Row> hnd = ctx.rowHandler();
- /** */
- private Comparator<Row> comparator(RelFieldCollation left, RelFieldCollation right) {
- final int nullComparison = left.nullDirection.nullComparison;
+ for (int i = 0; i < left.size(); i++) {
+ RelFieldCollation leftField = left.get(i);
+ RelFieldCollation rightField = right.get(i);
- if (nullComparison != right.nullDirection.nullComparison)
- throw new IllegalArgumentException("Can't be compared: left=" + left + ", right=" + right);
+ int lIdx = leftField.getFieldIndex();
+ int rIdx = rightField.getFieldIndex();
- final int lIdx = left.getFieldIndex();
- final int rIdx = right.getFieldIndex();
- RowHandler<Row> handler = ctx.rowHandler();
+ Object c1 = hnd.get(lIdx, o1);
+ Object c2 = hnd.get(rIdx, o2);
- if (left.direction != right.direction)
- throw new IllegalArgumentException("Can't be compared: left=" + left + ", right=" + right);
+ if (c1 == null && c2 == null) {
+ hasNulls = true;
+ continue;
+ }
- if (left.direction == RelFieldCollation.Direction.ASCENDING) {
- return (o1, o2) -> {
- final Object c1 = handler.get(lIdx, o1);
- final Object c2 = handler.get(rIdx, o2);
- return compare(c1, c2, nullComparison);
- };
- }
+ int nullComparison = leftField.nullDirection.nullComparison;
- return (o1, o2) -> {
- final Object c1 = handler.get(lIdx, o1);
- final Object c2 = handler.get(rIdx, o2);
- return compare(c2, c1, -nullComparison);
+ int res = leftField.direction == RelFieldCollation.Direction.ASCENDING ?
+ ExpressionFactoryImpl.compare(c1, c2, nullComparison) :
+ ExpressionFactoryImpl.compare(c2, c1, -nullComparison);
+
+ if (res != 0)
+ return res;
+ }
+
+ // If compared rows contain NULLs, they shouldn't be treated as equals, since NULL <> NULL in SQL.
+ return hasNulls ? 1 : 0;
+ }
};
}
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinExecutionTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinExecutionTest.java
index d5965a1..5f49702b 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinExecutionTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/MergeJoinExecutionTest.java
@@ -320,6 +320,62 @@ public class MergeJoinExecutionTest extends AbstractExecutionTest {
});
}
+ /** */
+ @Test
+ public void joinOnNullField() {
+ Object[][] left = {
+ {1, "Roman", null},
+ {2, "Igor", null},
+ {3, "Alexey", 1},
+ {4, "Ivan", 2}
+ };
+
+ Object[][] right = {
+ {null, "Core"},
+ {null, "OLD_Core"},
+ {1, "SQL"},
+ {2, "QA"}
+ };
+
+ verifyJoin(left, right, INNER, new Object[][] {
+ {3, "Alexey", "SQL"},
+ {4, "Ivan", "QA"},
+ });
+
+ verifyJoin(left, right, LEFT, new Object[][] {
+ {1, "Roman", null},
+ {2, "Igor", null},
+ {3, "Alexey", "SQL"},
+ {4, "Ivan", "QA"},
+ });
+
+ verifyJoin(left, right, RIGHT, new Object[][] {
+ {null, null, "Core"},
+ {null, null, "OLD_Core"},
+ {3, "Alexey", "SQL"},
+ {4, "Ivan", "QA"},
+ });
+
+ verifyJoin(left, right, FULL, new Object[][] {
+ {null, null, "Core"},
+ {null, null, "OLD_Core"},
+ {1, "Roman", null},
+ {2, "Igor", null},
+ {3, "Alexey", "SQL"},
+ {4, "Ivan", "QA"},
+ });
+
+ verifyJoin(left, right, SEMI, new Object[][] {
+ {3, "Alexey"},
+ {4, "Ivan"},
+ });
+
+ verifyJoin(left, right, ANTI, new Object[][] {
+ {1, "Roman"},
+ {2, "Igor"},
+ });
+ }
+
/**
* Creates execution tree and executes it. Then compares the result of the execution with the given one.
*
@@ -353,11 +409,12 @@ public class MergeJoinExecutionTest extends AbstractExecutionTest {
else if (o2 != null)
return -1;
else
- return 0;
+ return 1;
}
return Integer.compare((Integer)o1, (Integer)o2);
});
+
join.register(F.asList(leftNode, rightNode));
RelDataType rowType;
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/JoinIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/JoinIntegrationTest.java
index 84a7d55..d41bb01 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/JoinIntegrationTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/JoinIntegrationTest.java
@@ -229,6 +229,22 @@ public class JoinIntegrationTest extends AbstractBasicIntegrationTest {
.returns(4, 4, 4, 4, 4)
.returns(null, 3, 3, 3, 3)
.check();
+
+ assertQuery("" +
+ "select t1.c3 c13, t1.c2 c12, t1.c1 c11, t2.c1 c21, t2.c2 c22 " +
+ " from t1 " +
+ " join t2 " +
+ " on t1.c2 = t2.c2 " +
+ " order by t1.c3 nulls last, t1.c2 nulls last, t1.c1 nulls last"
+ )
+ .ordered()
+ .returns(1, 1, 1, 1, 1)
+ .returns(2, 2, 2, 2, 2)
+ .returns(2, 2, 2, 2, 2)
+ .returns(3, 3, 3, 3, 3)
+ .returns(4, 4, 4, 4, 4)
+ .returns(null, 3, 3, 3, 3)
+ .check();
}
/**
@@ -430,6 +446,23 @@ public class JoinIntegrationTest extends AbstractBasicIntegrationTest {
.returns(4, 4, 4, 4, 4)
.returns(null, 3, 3, 3, 3)
.check();
+
+ assertQuery("" +
+ "select t1.c3 c13, t1.c2 c12, t1.c1 c11, t2.c1 c21, t2.c2 c22 " +
+ " from t1 " +
+ " left join t2 " +
+ " on t1.c2 = t2.c2 " +
+ " order by t1.c3 nulls last, t1.c2 nulls last, t1.c1 nulls last"
+ )
+ .ordered()
+ .returns(1, 1, 1, 1, 1)
+ .returns(2, 2, 2, 2, 2)
+ .returns(2, 2, 2, 2, 2)
+ .returns(2, null, 2, null, null)
+ .returns(3, 3, 3, 3, 3)
+ .returns(4, 4, 4, 4, 4)
+ .returns(null, 3, 3, 3, 3)
+ .check();
}
/**
@@ -669,6 +702,50 @@ public class JoinIntegrationTest extends AbstractBasicIntegrationTest {
.returns(4, 4, 4, 4, 4, 4)
.returns(null, null, null, 2, 2, null)
.check();
+
+ assertQuery("" +
+ "select t1.c2 c12, t1.c1 c11, t2.c1 c21, t2.c2 c22, t2.c3 c23 " +
+ " from t1 " +
+ " right join t2 " +
+ " on t1.c2 = t2.c2 " +
+ " order by t2.c3 nulls last, t2.c2 nulls last, t2.c1 nulls last"
+ )
+ .ordered()
+ .returns(1, 1, 1, 1, 1)
+ .returns(2, 2, 2, 2, 2)
+ .returns(3, 3, 3, 3, 3)
+ .returns(3, 3, 3, 3, 3)
+ .returns(null, null, 3, null, 3)
+ .returns(4, 4, 4, 4, 4)
+ .returns(2, 2, 2, 2, null)
+ .check();
+ }
+
+ /**
+ * Test verifies result of full join.
+ */
+ @Test
+ public void testFullJoin() {
+ Assume.assumeTrue(joinType != JoinType.CORRELATED);
+
+ assertQuery("" +
+ "select t1.c1 c11, t1.c2 c12, t1.c3 c13, t2.c1 c21, t2.c2 c22, t2.c3 c23 " +
+ " from t1 " +
+ " full join t2 " +
+ " on t1.c2 = t2.c2 " +
+ " order by " +
+ " t1.c1 nulls last, t1.c2 nulls last, t1.c3 nulls last, t2.c1 nulls last, t2.c2 nulls last, t2.c3 nulls last"
+ )
+ .ordered()
+ .returns(1, 1, 1, 1, 1, 1)
+ .returns(2, 2, 2, 2, 2, 2)
+ .returns(2, 2, 2, 2, 2, null)
+ .returns(2, null, 2, null, null, null)
+ .returns(3, 3, 3, 3, 3, 3)
+ .returns(3, 3, null, 3, 3, 3)
+ .returns(4, 4, 4, 4, 4, 4)
+ .returns(null, null, null, 3, null, 3)
+ .check();
}
/**