Skip to content

Enable predicate pushdown for TIMESTAMP type in Delta Lake #12344

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

Merged
ebyhr merged 1 commit into from
May 19, 2022
Merged

Enable predicate pushdown for TIMESTAMP type in Delta Lake #12344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 1 addition & 3 deletions ...trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakePageSourceProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,7 @@ private static TupleDomain<HiveColumnHandle> getParquetTupleDomain(TupleDomain<D
effectivePredicate.getDomains().get().forEach((columnHandle, domain) -> {
String baseType = columnHandle.getType().getTypeSignature().getBase();
// skip looking up predicates for complex types as Parquet only stores stats for primitives
if (!baseType.equals(StandardTypes.MAP) && !baseType.equals(StandardTypes.ARRAY) && !baseType.equals(StandardTypes.ROW) &&
// TODO: Remove the next line once timestamp predicate pushdown works in Parquet reader (https://github.com/trinodb/trino/issues/12007)
!baseType.equals(StandardTypes.TIMESTAMP_WITH_TIME_ZONE)) {
Comment on lines 185 to 186
Copy link
Copy Markdown
Member

@findepi findepi May 13, 2022

Choose a reason for hiding this comment

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

@MiguelWeezardo i remember you originally added these lines

the TODO went with TestSplitPruning.testTimestampPruning
the condition went with some TestDeltaLakeReadTimestamps changes

please review

if (!baseType.equals(StandardTypes.MAP) && !baseType.equals(StandardTypes.ARRAY) && !baseType.equals(StandardTypes.ROW)) {
HiveColumnHandle hiveColumnHandle = columnHandle.toHiveColumnHandle();
predicate.put(hiveColumnHandle, domain);
}
Expand Down
48 changes: 48 additions & 0 deletions ...o-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeMinioConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import io.trino.execution.QueryInfo;
import io.trino.plugin.deltalake.util.DockerizedMinioDataLake;
import io.trino.testing.BaseConnectorTest;
import io.trino.testing.DistributedQueryRunner;
import io.trino.testing.MaterializedResult;
import io.trino.testing.QueryRunner;
import io.trino.testing.ResultWithQueryId;
import io.trino.testing.TestingConnectorBehavior;
import io.trino.testing.sql.TestTable;
import io.trino.tpch.TpchTable;
import org.testng.SkipException;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import java.util.Optional;
Expand Down Expand Up @@ -265,6 +269,50 @@ public void testCharVarcharComparison()
.hasStackTraceContaining("Unsupported type: char(3)");
}

@Test(dataProvider = "timestampValues")
public void testTimestampPredicatePushdown(String value)
{
String tableName = "test_parquet_timestamp_predicate_pushdown_" + randomTableSuffix();

assertUpdate("DROP TABLE IF EXISTS " + tableName);
assertUpdate("CREATE TABLE " + tableName + " (t TIMESTAMP WITH TIME ZONE)");
assertUpdate("INSERT INTO " + tableName + " VALUES (TIMESTAMP '" + value + "')", 1);

DistributedQueryRunner queryRunner = (DistributedQueryRunner) getQueryRunner();
ResultWithQueryId<MaterializedResult> queryResult = queryRunner.executeWithQueryId(
getSession(),
"SELECT * FROM " + tableName + " WHERE t < TIMESTAMP '" + value + "'");
assertEquals(getQueryInfo(queryRunner, queryResult).getQueryStats().getProcessedInputDataSize().toBytes(), 0);

queryResult = queryRunner.executeWithQueryId(
getSession(),
"SELECT * FROM " + tableName + " WHERE t > TIMESTAMP '" + value + "'");
assertEquals(getQueryInfo(queryRunner, queryResult).getQueryStats().getProcessedInputDataSize().toBytes(), 0);

assertQueryStats(
getSession(),
"SELECT * FROM " + tableName + " WHERE t = TIMESTAMP '" + value + "'",
queryStats -> assertThat(queryStats.getProcessedInputDataSize().toBytes()).isGreaterThan(0),
results -> {});
}

@DataProvider
public Object[][] timestampValues()
{
return new Object[][] {
{"1965-10-31 01:00:08.123 UTC"},
Copy link
Copy Markdown
Member

@MiguelWeezardo MiguelWeezardo May 16, 2022

Choose a reason for hiding this comment

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

Please add entries to test handling of gaps and ambiguous values in the JVM time zone. Note that these may appear as partition values as well.

Copy link
Copy Markdown
Member Author

@ebyhr ebyhr May 17, 2022

Choose a reason for hiding this comment

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

@MiguelWeezardo Added a below entry. Please take another look.

1970-01-01 01:13:42.000 America/Bahia_Banderas

{"1965-10-31 01:00:08.999 UTC"},
{"1970-01-01 01:13:42.000 America/Bahia_Banderas"}, // There is a gap in JVM zone
{"1970-01-01 00:00:00.000 Asia/Kathmandu"},
{"2018-10-28 01:33:17.456 Europe/Vilnius"},
{"9999-12-31 23:59:59.999 UTC"}};
}

private QueryInfo getQueryInfo(DistributedQueryRunner queryRunner, ResultWithQueryId<MaterializedResult> queryResult)
{
return queryRunner.getCoordinator().getQueryManager().getFullQueryInfo(queryResult.getQueryId());
}

@Override
protected String createSchemaSql(String schemaName)
{
Expand Down