Skip to content

Commit 9f52415

Browse files
Vladsz83alex-plekhanov
authored andcommitted
IGNITE-26048 SQL Calcite: Fix incorrect processing of IS NOT DISTINCT condition in MergeJoin - Fixes #12284.
Signed-off-by: Aleksey Plekhanov <[email protected]>
1 parent 43d8f02 commit 9f52415

File tree

7 files changed

+200
-25
lines changed

7 files changed

+200
-25
lines changed

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.function.Predicate;
2828
import java.util.function.Supplier;
2929
import org.apache.calcite.rel.RelCollation;
30+
import org.apache.calcite.rel.RelFieldCollation;
3031
import org.apache.calcite.rel.RelNode;
3132
import org.apache.calcite.rel.core.Intersect;
3233
import org.apache.calcite.rel.core.JoinRelType;
@@ -37,6 +38,7 @@
3738
import org.apache.calcite.rex.RexLiteral;
3839
import org.apache.calcite.rex.RexNode;
3940
import org.apache.calcite.util.ImmutableBitSet;
41+
import org.apache.calcite.util.mapping.IntPair;
4042
import org.apache.ignite.internal.processors.failure.FailureProcessor;
4143
import org.apache.ignite.internal.processors.query.QueryUtils;
4244
import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler.RowFactory;
@@ -116,8 +118,6 @@
116118
import org.apache.ignite.internal.util.typedef.F;
117119

118120
import static org.apache.calcite.rel.RelDistribution.Type.HASH_DISTRIBUTED;
119-
import static org.apache.calcite.sql.SqlKind.IS_DISTINCT_FROM;
120-
import static org.apache.calcite.sql.SqlKind.IS_NOT_DISTINCT_FROM;
121121
import static org.apache.ignite.internal.processors.query.calcite.util.TypeUtils.combinedRowType;
122122

123123
/**
@@ -298,12 +298,40 @@ public LogicalRelImplementor(
298298
RelDataType rightType = rel.getRight().getRowType();
299299
JoinRelType joinType = rel.getJoinType();
300300

301-
int pairsCnt = rel.analyzeCondition().pairs().size();
301+
List<IntPair> joinPairs = rel.analyzeCondition().pairs();
302+
int pairsCnt = joinPairs.size();
303+
304+
List<RelFieldCollation> leftCollations = rel.leftCollation().getFieldCollations();
305+
List<RelFieldCollation> rightCollations = rel.rightCollation().getFieldCollations();
306+
307+
ImmutableBitSet allowNulls = rel.allowNulls();
308+
ImmutableBitSet.Builder collsAllowNullsBuilder = ImmutableBitSet.builder();
309+
int lastCollField = -1;
310+
311+
for (int c = 0; c < Math.min(leftCollations.size(), rightCollations.size()); ++c) {
312+
RelFieldCollation leftColl = leftCollations.get(c);
313+
RelFieldCollation rightColl = rightCollations.get(c);
314+
collsAllowNullsBuilder.set(c);
315+
316+
for (int p = 0; p < pairsCnt; ++p) {
317+
IntPair pair = joinPairs.get(p);
318+
319+
if (pair.source == leftColl.getFieldIndex() && pair.target == rightColl.getFieldIndex()) {
320+
lastCollField = c;
321+
322+
if (!allowNulls.get(p)) {
323+
collsAllowNullsBuilder.clear(c);
324+
325+
break;
326+
}
327+
}
328+
}
329+
}
302330

303331
Comparator<Row> comp = expressionFactory.comparator(
304-
rel.leftCollation().getFieldCollations().subList(0, pairsCnt),
305-
rel.rightCollation().getFieldCollations().subList(0, pairsCnt),
306-
rel.getCondition().getKind() == IS_NOT_DISTINCT_FROM || rel.getCondition().getKind() == IS_DISTINCT_FROM
332+
leftCollations.subList(0, lastCollField + 1),
333+
rightCollations.subList(0, lastCollField + 1),
334+
collsAllowNullsBuilder.build()
307335
);
308336

309337
Node<Row> node = MergeJoinNode.create(ctx, outType, leftType, rightType, joinType, comp, hasExchange(rel));

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
import java.util.function.Function;
2424
import java.util.function.Predicate;
2525
import java.util.function.Supplier;
26-
2726
import org.apache.calcite.rel.RelCollation;
2827
import org.apache.calcite.rel.RelFieldCollation;
2928
import org.apache.calcite.rel.core.AggregateCall;
3029
import org.apache.calcite.rel.type.RelDataType;
3130
import org.apache.calcite.rex.RexLiteral;
3231
import org.apache.calcite.rex.RexNode;
32+
import org.apache.calcite.util.ImmutableBitSet;
3333
import org.apache.ignite.internal.processors.query.calcite.exec.exp.agg.AccumulatorWrapper;
3434
import org.apache.ignite.internal.processors.query.calcite.exec.exp.agg.AggregateType;
3535
import org.apache.ignite.internal.processors.query.calcite.prepare.bounds.SearchBounds;
@@ -60,11 +60,14 @@ Supplier<List<AccumulatorWrapper<Row>>> accumulatorsFactory(
6060
*
6161
* @param left Collations of left row.
6262
* @param right Collations of right row.
63-
* @param nullsEqual If {@code true}, nulls are considered equal. Usually, NULL <> NULL in SQL. So, the value should
64-
* be {@code false}. Except cases with IS DISTINCT / IS NOT DISTINCT.
63+
* @param allowNulls Matching null fields. Usually, NULL <> NULL in SQL. Except IS DISTINCT / IS NOT DISTINCT.
6564
* @return Rows comparator.
6665
*/
67-
Comparator<Row> comparator(List<RelFieldCollation> left, List<RelFieldCollation> right, boolean nullsEqual);
66+
Comparator<Row> comparator(
67+
List<RelFieldCollation> left,
68+
List<RelFieldCollation> right,
69+
ImmutableBitSet allowNulls
70+
);
6871

6972
/**
7073
* Creates a Filter predicate.

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.apache.calcite.rex.RexUtil;
6363
import org.apache.calcite.sql.type.SqlTypeName;
6464
import org.apache.calcite.sql.validate.SqlConformance;
65+
import org.apache.calcite.util.ImmutableBitSet;
6566
import org.apache.ignite.internal.binary.BinaryUtils;
6667
import org.apache.ignite.internal.processors.query.calcite.exec.ExecutionContext;
6768
import org.apache.ignite.internal.processors.query.calcite.exec.RowHandler;
@@ -197,7 +198,11 @@ else if (c2 != HIGHEST_VALUE)
197198
}
198199

199200
/** {@inheritDoc} */
200-
@Override public Comparator<Row> comparator(List<RelFieldCollation> left, List<RelFieldCollation> right, boolean nullsEqual) {
201+
@Override public Comparator<Row> comparator(
202+
List<RelFieldCollation> left,
203+
List<RelFieldCollation> right,
204+
ImmutableBitSet allowNulls
205+
) {
201206
if (F.isEmpty(left) || F.isEmpty(right) || left.size() != right.size())
202207
throw new IllegalArgumentException("Both inputs should be non-empty and have the same size: left="
203208
+ (left != null ? left.size() : "null") + ", right=" + (right != null ? right.size() : "null"));
@@ -213,9 +218,10 @@ else if (c2 != HIGHEST_VALUE)
213218

214219
return new Comparator<Row>() {
215220
@Override public int compare(Row o1, Row o2) {
216-
boolean hasNulls = false;
217221
RowHandler<Row> hnd = ctx.rowHandler();
218222

223+
boolean hasNulls = false;
224+
219225
for (int i = 0; i < left.size(); i++) {
220226
RelFieldCollation leftField = left.get(i);
221227
RelFieldCollation rightField = right.get(i);
@@ -226,8 +232,9 @@ else if (c2 != HIGHEST_VALUE)
226232
Object c1 = hnd.get(lIdx, o1);
227233
Object c2 = hnd.get(rIdx, o2);
228234

229-
if (c1 == null && c2 == null) {
230-
hasNulls = true;
235+
if (c1 == null && c2 == null && !hasNulls) {
236+
hasNulls = !allowNulls.get(i);
237+
231238
continue;
232239
}
233240

@@ -243,7 +250,7 @@ else if (c2 != HIGHEST_VALUE)
243250

244251
// If compared rows contain NULLs, they shouldn't be treated as equals, since NULL <> NULL in SQL.
245252
// Except cases with IS DISTINCT / IS NOT DISTINCT.
246-
return hasNulls && !nullsEqual ? 1 : 0;
253+
return hasNulls ? 1 : 0;
247254
}
248255
};
249256
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.ignite.internal.processors.query.calcite.rel;
19+
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
import com.google.common.collect.ImmutableList;
23+
import org.apache.calcite.plan.RelOptUtil;
24+
import org.apache.calcite.rel.core.Join;
25+
import org.apache.calcite.rel.core.JoinInfo;
26+
import org.apache.calcite.rex.RexNode;
27+
import org.apache.calcite.util.ImmutableBitSet;
28+
import org.apache.calcite.util.ImmutableIntList;
29+
30+
/** Extended {@link JoinInfo}. */
31+
public class IgniteJoinInfo extends JoinInfo {
32+
/** Conditions with mathing nulls. It usually means presence of 'IS DISTINCT' / 'IS NOT DISTINCT'. */
33+
private final ImmutableBitSet allowNulls;
34+
35+
/** */
36+
protected IgniteJoinInfo(
37+
ImmutableIntList leftKeys,
38+
ImmutableIntList rightKeys,
39+
ImmutableBitSet allowNulls,
40+
ImmutableList<RexNode> nonEquis
41+
) {
42+
super(leftKeys, rightKeys, nonEquis);
43+
44+
this.allowNulls = allowNulls;
45+
}
46+
47+
/** */
48+
public static IgniteJoinInfo of(Join join) {
49+
List<Integer> leftKeys = new ArrayList<>();
50+
List<Integer> rightKeys = new ArrayList<>();
51+
List<Boolean> skipNulls = new ArrayList<>();
52+
List<RexNode> nonEquis = new ArrayList<>();
53+
54+
RelOptUtil.splitJoinCondition(join.getLeft(), join.getRight(), join.getCondition(), leftKeys, rightKeys,
55+
skipNulls, nonEquis);
56+
57+
ImmutableBitSet.Builder allowNulls = ImmutableBitSet.builder();
58+
59+
for (int i = 0; i < skipNulls.size(); ++i) {
60+
if (!skipNulls.get(i))
61+
allowNulls.set(i);
62+
}
63+
64+
return new IgniteJoinInfo(
65+
ImmutableIntList.copyOf(leftKeys),
66+
ImmutableIntList.copyOf(rightKeys),
67+
allowNulls == null ? ImmutableBitSet.of() : ImmutableBitSet.of(allowNulls.build()),
68+
ImmutableList.copyOf(nonEquis)
69+
);
70+
}
71+
72+
/** */
73+
public ImmutableBitSet allowNulls() {
74+
return allowNulls;
75+
}
76+
}

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteMergeJoin.java

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.ArrayList;
2121
import java.util.List;
2222
import java.util.Set;
23-
2423
import com.google.common.collect.ImmutableList;
2524
import com.google.common.collect.ImmutableSet;
2625
import org.apache.calcite.plan.RelOptCluster;
@@ -45,6 +44,7 @@
4544
import org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCostFactory;
4645
import org.apache.ignite.internal.processors.query.calcite.trait.TraitUtils;
4746
import org.apache.ignite.internal.processors.query.calcite.util.Commons;
47+
import org.jetbrains.annotations.Nullable;
4848

4949
import static org.apache.calcite.rel.RelCollations.EMPTY;
5050
import static org.apache.calcite.rel.RelCollations.containsOrderless;
@@ -64,17 +64,21 @@ public class IgniteMergeJoin extends AbstractIgniteJoin {
6464
*/
6565
private final RelCollation rightCollation;
6666

67+
/** Mathing null conditions like 'IS NOT DISTINCT'. */
68+
private final ImmutableBitSet allowNulls;
69+
6770
/** */
6871
public IgniteMergeJoin(
6972
RelOptCluster cluster,
7073
RelTraitSet traitSet,
7174
RelNode left,
7275
RelNode right,
7376
RexNode condition,
77+
@Nullable ImmutableBitSet allowNulls,
7478
Set<CorrelationId> variablesSet,
7579
JoinRelType joinType
7680
) {
77-
this(cluster, traitSet, left, right, condition, variablesSet, joinType,
81+
this(cluster, traitSet, left, right, condition, allowNulls, variablesSet, joinType,
7882
left.getTraitSet().getCollation(), right.getTraitSet().getCollation());
7983
}
8084

@@ -86,6 +90,7 @@ public IgniteMergeJoin(RelInput input) {
8690
input.getInputs().get(0),
8791
input.getInputs().get(1),
8892
input.getExpression("condition"),
93+
input.getBitSet("allowNulls"),
8994
ImmutableSet.copyOf(Commons.transform(input.getIntegerList("variablesSet"), CorrelationId::new)),
9095
input.getEnum("joinType", JoinRelType.class),
9196
((RelInputEx)input).getCollation("leftCollation"),
@@ -100,6 +105,7 @@ private IgniteMergeJoin(
100105
RelNode left,
101106
RelNode right,
102107
RexNode condition,
108+
@Nullable ImmutableBitSet allowNulls,
103109
Set<CorrelationId> variablesSet,
104110
JoinRelType joinType,
105111
RelCollation leftCollation,
@@ -109,12 +115,13 @@ private IgniteMergeJoin(
109115

110116
this.leftCollation = leftCollation;
111117
this.rightCollation = rightCollation;
118+
this.allowNulls = allowNulls == null ? ImmutableBitSet.of() : allowNulls;
112119
}
113120

114121
/** {@inheritDoc} */
115122
@Override public Join copy(RelTraitSet traitSet, RexNode condition, RelNode left, RelNode right,
116123
JoinRelType joinType, boolean semiJoinDone) {
117-
return new IgniteMergeJoin(getCluster(), traitSet, left, right, condition, variablesSet, joinType,
124+
return new IgniteMergeJoin(getCluster(), traitSet, left, right, condition, allowNulls, variablesSet, joinType,
118125
left.getTraitSet().getCollation(), right.getTraitSet().getCollation());
119126
}
120127

@@ -125,7 +132,7 @@ private IgniteMergeJoin(
125132

126133
/** {@inheritDoc} */
127134
@Override public IgniteRel clone(RelOptCluster cluster, List<IgniteRel> inputs) {
128-
return new IgniteMergeJoin(cluster, getTraitSet(), inputs.get(0), inputs.get(1), getCondition(),
135+
return new IgniteMergeJoin(cluster, getTraitSet(), inputs.get(0), inputs.get(1), getCondition(), allowNulls,
129136
getVariablesSet(), getJoinType(), leftCollation, rightCollation);
130137
}
131138

@@ -270,7 +277,8 @@ else if (containsOrderless(rightKeys, collation)) {
270277
@Override public RelWriter explainTerms(RelWriter pw) {
271278
return super.explainTerms(pw)
272279
.item("leftCollation", leftCollation)
273-
.item("rightCollation", rightCollation);
280+
.item("rightCollation", rightCollation)
281+
.item("allowNulls", allowNulls);
274282
}
275283

276284
/**
@@ -287,6 +295,13 @@ public RelCollation rightCollation() {
287295
return rightCollation;
288296
}
289297

298+
/**
299+
* @return Matching null conditions.
300+
*/
301+
public ImmutableBitSet allowNulls() {
302+
return allowNulls;
303+
}
304+
290305
/** Creates pair with default collation for parent and simple yet sufficient collation for children nodes. */
291306
private Pair<RelTraitSet, List<RelTraitSet>> defaultCollationPair(
292307
RelTraitSet nodeTraits,

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/MergeJoinConverterRule.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@
2525
import org.apache.calcite.rel.PhysicalNode;
2626
import org.apache.calcite.rel.RelCollations;
2727
import org.apache.calcite.rel.RelNode;
28-
import org.apache.calcite.rel.core.JoinInfo;
2928
import org.apache.calcite.rel.logical.LogicalJoin;
3029
import org.apache.calcite.rel.metadata.RelMetadataQuery;
3130
import org.apache.ignite.internal.processors.query.calcite.hint.HintDefinition;
3231
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteConvention;
32+
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteJoinInfo;
3333
import org.apache.ignite.internal.processors.query.calcite.rel.IgniteMergeJoin;
3434
import org.apache.ignite.internal.util.typedef.F;
3535

@@ -51,16 +51,18 @@ public MergeJoinConverterRule() {
5151
@Override public boolean matchesJoin(RelOptRuleCall call) {
5252
LogicalJoin logicalJoin = call.rel(0);
5353

54-
JoinInfo joinInfo = JoinInfo.of(logicalJoin.getLeft(), logicalJoin.getRight(), logicalJoin.getCondition());
54+
IgniteJoinInfo info = IgniteJoinInfo.of(logicalJoin);
5555

56-
return !F.isEmpty(joinInfo.pairs()) && joinInfo.isEqui();
56+
return info.isEqui() && !F.isEmpty(info.pairs());
5757
}
5858

5959
/** {@inheritDoc} */
6060
@Override protected PhysicalNode convert(RelOptPlanner planner, RelMetadataQuery mq, LogicalJoin rel) {
6161
RelOptCluster cluster = rel.getCluster();
6262

63-
JoinInfo joinInfo = JoinInfo.of(rel.getLeft(), rel.getRight(), rel.getCondition());
63+
IgniteJoinInfo joinInfo = IgniteJoinInfo.of(rel);
64+
65+
assert joinInfo.isEqui() && !F.isEmpty(joinInfo.pairs());
6466

6567
RelTraitSet leftInTraits = cluster.traitSetOf(IgniteConvention.INSTANCE)
6668
.replace(RelCollations.of(joinInfo.leftKeys));
@@ -71,6 +73,7 @@ public MergeJoinConverterRule() {
7173
RelNode left = convert(rel.getLeft(), leftInTraits);
7274
RelNode right = convert(rel.getRight(), rightInTraits);
7375

74-
return new IgniteMergeJoin(cluster, outTraits, left, right, rel.getCondition(), rel.getVariablesSet(), rel.getJoinType());
76+
return new IgniteMergeJoin(cluster, outTraits, left, right, rel.getCondition(), joinInfo.allowNulls(),
77+
rel.getVariablesSet(), rel.getJoinType());
7578
}
7679
}

0 commit comments

Comments
 (0)