Skip to content

Commit f96c605

Browse files
codlucacodluca
committed
Fix 'IS NOT DISTINCT FROM' not working in JOIN when nulls are used
The changes try to enable Domain with only null value to be used. Modify JoinDomainBuilder to take into consideration the nulls from the value blocks.
1 parent 5a57103 commit f96c605

File tree

4 files changed

+160
-11
lines changed

4 files changed

+160
-11
lines changed

‎core/trino-main/src/main/java/io/trino/operator/JoinDomainBuilder.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,14 @@ public class JoinDomainBuilder
9999

100100
private long retainedSizeInBytes = INSTANCE_SIZE;
101101

102+
/**
103+
* Indicates whether null values are allowed in the join domain.
104+
* This is set to true if any null values are observed in the input blocks
105+
* during domain building, and is used to determine whether the resulting
106+
* domain should include nulls.
107+
*/
108+
private boolean nullsAllowed;
109+
102110
public JoinDomainBuilder(
103111
Type type,
104112
int maxDistinctValues,
@@ -160,6 +168,9 @@ public boolean isCollecting()
160168

161169
public void add(Block block)
162170
{
171+
if (block.hasNull()) {
172+
nullsAllowed = true;
173+
}
163174
if (collectDistinctValues) {
164175
switch (block) {
165176
case ValueBlock valueBlock -> {
@@ -290,8 +301,7 @@ public Domain build()
290301
}
291302
}
292303
}
293-
// Inner and right join doesn't match rows with null key column values.
294-
return Domain.create(ValueSet.copyOf(type, values.build()), false);
304+
return Domain.create(ValueSet.copyOf(type, values.build()), nullsAllowed);
295305
}
296306
if (collectMinMax) {
297307
if (minValue == null) {
@@ -307,7 +317,6 @@ public Domain build()
307317

308318
private void add(ValueBlock block, int position)
309319
{
310-
// Inner and right join doesn't match rows with null key column values.
311320
if (block.isNull(position)) {
312321
return;
313322
}

‎core/trino-main/src/main/java/io/trino/sql/DynamicFilters.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -295,19 +295,18 @@ public Domain applyComparison(Domain domain)
295295
if (domain.isAll()) {
296296
return domain;
297297
}
298-
if (domain.isNone()) {
299-
// Dynamic filter collection skips nulls
298+
if (domain.getValues().isNone()) {
300299
// In case of IS NOT DISTINCT FROM, an empty Domain should still allow null
301300
if (nullAllowed) {
302301
return Domain.onlyNull(domain.getType());
303302
}
304-
return domain;
303+
return Domain.none(domain.getType());
305304
}
306305
Range span = domain.getValues().getRanges().getSpan();
307306
return switch (operator) {
308307
case EQUAL -> {
309-
if (nullAllowed) {
310-
yield Domain.create(domain.getValues(), true);
308+
if (nullAllowed != domain.isNullAllowed()) {
309+
yield Domain.create(domain.getValues(), nullAllowed);
311310
}
312311
yield domain;
313312
}

‎core/trino-main/src/test/java/io/trino/operator/TestDynamicFilterSourceOperator.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ public void testCollectWithNulls()
287287

288288
assertThat(partitions.build()).isEqualTo(ImmutableList.of(
289289
TupleDomain.withColumnDomains(ImmutableMap.of(
290-
new DynamicFilterId("0"), Domain.create(ValueSet.of(INTEGER, 1L, 2L, 3L, 4L, 5L), false)))));
290+
new DynamicFilterId("0"), Domain.create(ValueSet.of(INTEGER, 1L, 2L, 3L, 4L, 5L), true)))));
291291
}
292292

293293
@Test
@@ -490,7 +490,13 @@ public void testMultipleColumnsCollectMinMaxWithNulls()
490490
maxDistinctValues,
491491
ImmutableList.of(BIGINT, BIGINT),
492492
ImmutableList.of(largePage),
493-
ImmutableList.of(TupleDomain.none()));
493+
ImmutableList.of(TupleDomain.withColumnDomains(ImmutableMap.of(
494+
new DynamicFilterId("0"),
495+
Domain.onlyNull(BIGINT),
496+
new DynamicFilterId("1"),
497+
Domain.create(
498+
ValueSet.ofRanges(range(BIGINT, 200L, true, 300L, true)),
499+
false)))));
494500
}
495501

496502
@Test
@@ -570,7 +576,7 @@ public void testCollectDeduplication()
570576
ImmutableList.of(largePage, nullsPage),
571577
ImmutableList.of(TupleDomain.withColumnDomains(ImmutableMap.of(
572578
new DynamicFilterId("0"),
573-
Domain.create(ValueSet.of(BIGINT, 7L), false)))));
579+
Domain.create(ValueSet.of(BIGINT, 7L), true)))));
574580
}
575581

576582
@Test

‎testing/trino-tests/src/test/java/io/trino/tests/TestJoinIsNotDistinct.java‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package io.trino.tests;
15+
16+
import com.google.common.collect.ImmutableMap;
17+
import io.trino.plugin.memory.MemoryPlugin;
18+
import io.trino.sql.query.QueryAssertions;
19+
import io.trino.testing.QueryRunner;
20+
import io.trino.testing.StandaloneQueryRunner;
21+
import org.junit.jupiter.api.AfterAll;
22+
import org.junit.jupiter.api.Test;
23+
import org.junit.jupiter.api.TestInstance;
24+
import org.junit.jupiter.api.parallel.Execution;
25+
import org.junit.jupiter.api.parallel.ExecutionMode;
26+
27+
import static io.trino.testing.TestingNames.randomNameSuffix;
28+
import static io.trino.testing.TestingSession.testSessionBuilder;
29+
import static java.lang.String.format;
30+
import static org.assertj.core.api.Assertions.assertThat;
31+
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
32+
33+
@TestInstance(PER_CLASS)
34+
@Execution(ExecutionMode.SAME_THREAD)
35+
public class TestJoinIsNotDistinct
36+
{
37+
private static final String LOCAL_CATALOG = "local";
38+
private static final String DEFAULT_SCHEMA = "default";
39+
40+
private final QueryRunner queryRunner;
41+
42+
private final QueryAssertions assertions;
43+
44+
public TestJoinIsNotDistinct()
45+
{
46+
queryRunner = new StandaloneQueryRunner(testSessionBuilder()
47+
.setCatalog(LOCAL_CATALOG)
48+
.setSchema(DEFAULT_SCHEMA)
49+
.build());
50+
queryRunner.installPlugin(new MemoryPlugin());
51+
queryRunner.createCatalog(LOCAL_CATALOG, "memory", ImmutableMap.of());
52+
53+
assertions = new QueryAssertions(queryRunner);
54+
}
55+
56+
@AfterAll
57+
public void teardown()
58+
{
59+
assertions.close();
60+
}
61+
62+
@Test
63+
public void testJoinWithIsNotDistinctFromOnNulls()
64+
{
65+
String tableName1 = "test_tab_" + randomNameSuffix();
66+
String tableName2 = "test_tab_" + randomNameSuffix();
67+
queryRunner.execute(format("CREATE TABLE %s (k1 INT, k2 INT)", tableName1));
68+
queryRunner.execute(format("CREATE TABLE %s (k1 INT, k2 INT)", tableName2));
69+
70+
queryRunner.execute(format("INSERT INTO %s VALUES (1, NULL)", tableName1));
71+
queryRunner.execute(format("INSERT INTO %s VALUES (1, NULL)", tableName2));
72+
assertThat(assertions.query(format("SELECT *" +
73+
" FROM %s t" +
74+
" INNER JOIN %s AS s" +
75+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
76+
" AND s.k2 IS NOT DISTINCT FROM t.k2", tableName1, tableName2)))
77+
.matches("VALUES (1, CAST(NULL AS INTEGER), 1, CAST(NULL AS INTEGER))");
78+
79+
queryRunner.execute(format("INSERT INTO %s VALUES (NULL, NULL)", tableName1));
80+
queryRunner.execute(format("INSERT INTO %s VALUES (NULL, NULL)", tableName2));
81+
assertThat(assertions.query(format("SELECT *" +
82+
" FROM %s t" +
83+
" INNER JOIN %s AS s" +
84+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
85+
" AND s.k2 IS NOT DISTINCT FROM t.k2", tableName1, tableName2)))
86+
.matches("VALUES (1, CAST(NULL AS INTEGER), 1, CAST(NULL AS INTEGER))," +
87+
" (CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER))");
88+
89+
queryRunner.execute(format("INSERT INTO %s VALUES (NULL, 2)", tableName1));
90+
queryRunner.execute(format("INSERT INTO %s VALUES (3, NULL)", tableName2));
91+
assertThat(assertions.query(format("SELECT *" +
92+
" FROM %s t" +
93+
" INNER JOIN %s AS s" +
94+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
95+
" AND s.k2 IS NOT DISTINCT FROM t.k2", tableName1, tableName2)))
96+
.matches("VALUES (1, CAST(NULL AS INTEGER), 1, CAST(NULL AS INTEGER))," +
97+
" (CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER))");
98+
99+
queryRunner.execute(format("INSERT INTO %s VALUES (2, 2)", tableName1));
100+
queryRunner.execute(format("INSERT INTO %s VALUES (2, 2)", tableName2));
101+
assertThat(assertions.query(format("SELECT *" +
102+
" FROM %s t" +
103+
" INNER JOIN %s AS s" +
104+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
105+
" AND s.k2 IS NOT DISTINCT FROM t.k2", tableName1, tableName2)))
106+
.matches("VALUES (1, CAST(NULL AS INTEGER), 1, CAST(NULL AS INTEGER))," +
107+
" (CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER))," +
108+
" (2, 2, 2, 2)");
109+
}
110+
111+
@Test
112+
public void testJoinWithIsNotDistinctFromOnNullsOnDerivedTables()
113+
{
114+
assertThat(assertions.query("SELECT *" +
115+
" FROM (SELECT 1 AS k1, CAST(NULL AS INTEGER) AS k2) t" +
116+
" INNER JOIN (SELECT 1 AS k1, CAST(NULL AS INTEGER) AS k2) AS s" +
117+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
118+
" AND s.k2 IS NOT DISTINCT FROM t.k2"))
119+
.matches("VALUES (1, CAST(NULL AS INTEGER), 1, CAST(NULL AS INTEGER))");
120+
121+
assertThat(assertions.query("SELECT *" +
122+
" FROM (SELECT CAST(NULL AS INTEGER) AS k1, CAST(NULL AS INTEGER) AS k2) t" +
123+
" INNER JOIN (SELECT CAST(NULL AS INTEGER) AS k1, CAST(NULL AS INTEGER) AS k2) AS s" +
124+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
125+
" AND s.k2 IS NOT DISTINCT FROM t.k2"))
126+
.matches("VALUES (CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER), CAST(NULL AS INTEGER))");
127+
128+
assertThat(assertions.query("SELECT *" +
129+
" FROM (SELECT CAST(NULL AS INTEGER) AS k1, 2 AS k2) t" +
130+
" INNER JOIN (SELECT 3 AS k1, CAST(NULL AS INTEGER) AS k2) AS s" +
131+
" ON s.k1 IS NOT DISTINCT FROM t.k1" +
132+
" AND s.k2 IS NOT DISTINCT FROM t.k2"))
133+
.returnsEmptyResult();
134+
}
135+
}

0 commit comments

Comments
 (0)