Skip to content

Pushdown DATE, TIMESTAMP, and TIMESTAMP_TZ_MILLIS predicates in MongoDB #13517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
harlequin wants to merge 4 commits into from
Closed

Pushdown DATE, TIMESTAMP, and TIMESTAMP_TZ_MILLIS predicates in MongoDB #13517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbd24b4
Add path from pr #5682
harlequin Aug 4, 2022
75b88ad
Changed Long to long
harlequin Aug 5, 2022
0de8cbd
Merge base pr
harlequin Aug 5, 2022
59ba425
Add TypeTests
harlequin Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

Expand All @@ -78,9 +79,12 @@
import static io.trino.spi.HostAddress.fromParts;
import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.spi.type.BooleanType.BOOLEAN;
import static io.trino.spi.type.DateTimeEncoding.unpackMillisUtc;
import static io.trino.spi.type.DateType.DATE;
import static io.trino.spi.type.DoubleType.DOUBLE;
import static io.trino.spi.type.SmallintType.SMALLINT;
import static io.trino.spi.type.TimestampType.TIMESTAMP_MILLIS;
import static io.trino.spi.type.TimestampWithTimeZoneType.TIMESTAMP_TZ_MILLIS;
import static io.trino.spi.type.TinyintType.TINYINT;
import static io.trino.spi.type.VarbinaryType.VARBINARY;
import static io.trino.spi.type.VarcharType.VARCHAR;
Expand Down Expand Up @@ -508,6 +512,18 @@ private static Optional<Object> translateValue(Object trinoNativeValue, Type typ
return Optional.of(trinoNativeValue);
}

if (type == DATE) {
return Optional.of(new Date(TimeUnit.DAYS.toMillis((Long) trinoNativeValue)));
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend adding a preparatory commit to use LocalDate for DATE type in other places.

}

if (type == TIMESTAMP_MILLIS) {
return Optional.of(new Date(TimeUnit.MILLISECONDS.convert((Long) trinoNativeValue, TimeUnit.MICROSECONDS)));
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use LocalDateTime

}

if (type == TIMESTAMP_TZ_MILLIS) {
return Optional.of(new Date(unpackMillisUtc((long) trinoNativeValue)));
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use LocalDateTime

}

if (type instanceof ObjectIdType) {
return Optional.of(new ObjectId(((Slice) trinoNativeValue).getBytes()));
}
Expand Down
36 changes: 36 additions & 0 deletions plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/BaseMongoConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.mongodb.DBRef;
import com.mongodb.client.MongoClient;
import com.mongodb.client.MongoCollection;
import io.trino.sql.planner.plan.FilterNode;
import io.trino.sql.planner.plan.LimitNode;
import io.trino.testing.BaseConnectorTest;
import io.trino.testing.MaterializedResult;
Expand Down Expand Up @@ -589,6 +590,41 @@ protected void verifyTableNameLengthFailurePermissible(Throwable e)
assertThat(e).hasMessageMatching(".*fully qualified namespace .* is too long.*");
}

@Test
public void testPredicatePushdown()
{
// TODO test that that predicate is actually pushed down (here we test only correctness)
// varchar equality
assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name = 'ROMANIA'"))
.matches("VALUES (BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar(25)))")
.isNotFullyPushedDown(FilterNode.class);

// varchar range
assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name BETWEEN 'POLAND' AND 'RPA'"))
.matches("VALUES (BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar(25)))")
.isNotFullyPushedDown(FilterNode.class);

// varchar different case
assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE name = 'romania'"))
.returnsEmptyResult()
.isNotFullyPushedDown(FilterNode.class);

// bigint equality
assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE nationkey = 19"))
.matches("VALUES (BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar(25)))")
.isNotFullyPushedDown(FilterNode.class);

// bigint range, with decimal to bigint simplification
assertThat(query("SELECT regionkey, nationkey, name FROM nation WHERE nationkey BETWEEN 18.5 AND 19.5"))
.matches("VALUES (BIGINT '3', BIGINT '19', CAST('ROMANIA' AS varchar(25)))")
.isNotFullyPushedDown(FilterNode.class);
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests unrelates to date, timestamp, timestamp_tz_millis change. Please separate a commit.


// date equality
assertThat(query("SELECT orderkey FROM orders WHERE orderdate = DATE '1992-09-29'"))
.matches("VALUES BIGINT '1250', 34406, 38436, 57570")
.isNotFullyPushedDown(FilterNode.class);
}

private void assertOneNotNullResult(String query)
{
MaterializedResult results = getQueryRunner().execute(getSession(), query).toTestTypes();
Expand Down
3 changes: 3 additions & 0 deletions plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoTypeMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ public void testTime()
.addRoundTrip("time", "TIME '23:59:59.9'", createTimeType(3), "TIME '23:59:59.900'")
.addRoundTrip("time", "TIME '23:59:59.99'", createTimeType(3), "TIME '23:59:59.990'")
.addRoundTrip("time", "TIME '23:59:59.999'", createTimeType(3), "TIME '23:59:59.999'")
.addRoundTrip("time", "TIME '23:59:59.9999'", createTimeType(4), "TIME '23:59:59.9999'")
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests for timestamp and timestamp with time zone types.

.addRoundTrip("time", "TIME '23:59:59.99999'", createTimeType(5), "TIME '23:59:59.99999'")
.addRoundTrip("time", "TIME '23:59:59.999999'", createTimeType(6), "TIME '23:59:59.999999'")
Copy link
Copy Markdown
Member

@ebyhr ebyhr Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check CI failures.

.execute(getQueryRunner(), trinoCreateAsSelect("test_time"))
.execute(getQueryRunner(), trinoCreateAndInsert("test_time"));
}
Expand Down