Push down predicate involving timestamp_tz in Delta Lake #19874

kasiafi · 2023-11-23T11:36:47Z

Description

Certain expressions involving TIMESTAMP WITH TIME ZONE can be optimized in Delta Lake connector based on the fact that columns of type TIMESTAMP WITH TIME ZONE are represented using the UTC time zone:

date_trunc('...', timeztamp_tz_column) <comparison> constant
CAST(timestamp_tz_column AS DATE) <comparison> constant
year(timestamp_tz_column) <comparison> constant

The optimization is based on similar optimization in the Iceberg connector.

Fixes #18664

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Delta Lake connector
* Pushdown filters involving columns of type TIMESTAMP WITH TIME ZONE. ({issue}`18664`)

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/ConstraintExtractor.java

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/UtcConstraintExtractor.java

core/trino-main/src/test/java/io/trino/sql/query/QueryAssertions.java

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/filter/UtcConstraintExtractor.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestUtcConstraintExtractor.java

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestConstraintExtractor.java

Rename ConstraintExtractor to UtcConstraintExtractor

findinpath · 2023-11-26T04:28:23Z

...ino-plugin-toolkit/src/test/java/io/trino/plugin/base/filter/TestUtcConstraintExtractor.java

-     * {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. Within Iceberg, we know
+     * Test equivalent of {@code io.trino.sql.planner.iterative.rule.UnwrapCastInComparison} for {@link TimestampWithTimeZoneType}.
+     * {@code UnwrapCastInComparison} handles {@link DateType} and {@link TimestampType}, but cannot handle
+     * {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. If we know


Suggested change

* {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. If we know

* {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. We know

same for the other occurences

If the "if" was removed, it would suggest that this is always true that the time zone is UTC. It can be only guaranteed per connector, and the optimization only applies to those connectors.

findinpath · 2023-11-26T04:42:57Z

...ino-plugin-toolkit/src/test/java/io/trino/plugin/base/filter/TestUtcConstraintExtractor.java

+        long startOfDateUtcEpochMillis = someDate.atStartOfDay().toEpochSecond(UTC) * MILLISECONDS_PER_SECOND;
+        long startOfDateUtc = timestampTzMillisFromEpochMillis(startOfDateUtcEpochMillis);
+        long startOfNextDateUtc = timestampTzMillisFromEpochMillis(startOfDateUtcEpochMillis + MILLISECONDS_PER_DAY);
+


Reduce code redundancy with the method testExtractTimestampTzMicrosDateComparison

When writing this, I experimented with extracting common code for similar test methods. I eventually settled for more repetition, as it improves readability a lot.

findinpath · 2023-11-26T05:20:50Z

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java

+                .matches("VALUES " +
+                        "(3, TIMESTAMP '2023-11-21 07:19:00.000 UTC')," +
+                        "(4, TIMESTAMP '2005-09-10 13:00:00.000 UTC')");
+


optional: This assertion can be added as well.

assertThat(query("SELECT * FROM " + tableName + " WHERE date_trunc('hour', part) >= TIMESTAMP '2005-09-10 00:00:00.001 +07:00'")) .isFullyPushedDown();

Added a similar test case, thanks!

… all precisions up to nanos

mosabua · 2023-11-27T16:54:14Z

@kasiafi and others... do we need to add a pushdown section to the performance section in the Delta Lake connector docs .. some here https://trino.io/docs/current/connector/delta-lake.html#performance

Similar to how we do it for JDBC connectors like https://trino.io/docs/current/connector/postgresql.html#pushdown

findinpath · 2023-11-28T11:50:24Z

do we need to add a pushdown section to the performance section in the Delta Lake connector docs ?

I tend to say no - the improvemenst covered by this PR (which btw exist for Iceberg connector as well) are goodies that should just be there - they are "obvious" from user perspective.

While we were working on these topics on Iceberg, we've published a blog post about them https://trino.io/blog/2023/04/11/date-predicates.html

findepi · 2023-11-28T11:58:22Z

goodies that should just be there - they are "obvious" from user perspective.

💯 user docs is not the place to brag about various optimization techniques we are using.
for example, we don't document all the optimizer rules that we ship

mosabua · 2023-11-28T19:13:22Z

Okay .. thanks @findepi and @findinpath

cla-bot bot added the cla-signed label Nov 23, 2023

github-actions bot added iceberg Iceberg connector delta-lake Delta Lake connector labels Nov 23, 2023

kasiafi force-pushed the 508OptimizeComparisonTimestampTz branch from 86b1abf to 71003a0 Compare November 23, 2023 12:01

kasiafi requested review from findinpath and ebyhr November 23, 2023 12:46

findepi approved these changes Nov 24, 2023

View reviewed changes

kasiafi added 3 commits November 24, 2023 21:00

Remove dependency on trino-main from TestConstraintExtractor

723527e

Generalize ConstraintExtractor to use arbitrary ColumnHandle

8402cfe

Move ConstraintExtractor to plugin-toolkit

3c3157f

Rename ConstraintExtractor to UtcConstraintExtractor

kasiafi force-pushed the 508OptimizeComparisonTimestampTz branch 2 times, most recently from 25e9cee to 869594b Compare November 24, 2023 21:21

findinpath reviewed Nov 26, 2023

View reviewed changes

kasiafi added 2 commits November 27, 2023 08:34

Generalize ConstraintExtractor to handle TimestempWithTimezoneType of…

b60c9b5

… all precisions up to nanos

Add UTC-based optimizations in DeltaLakeMetadata

3964149

kasiafi force-pushed the 508OptimizeComparisonTimestampTz branch from 869594b to 3964149 Compare November 27, 2023 08:14

kasiafi merged commit 69759e6 into trinodb:master Nov 27, 2023

github-actions bot added this to the 434 milestone Nov 27, 2023

mosabua mentioned this pull request Nov 27, 2023

Add Trino 434 release notes #19764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Push down predicate involving timestamp_tz in Delta Lake #19874

Push down predicate involving timestamp_tz in Delta Lake #19874

kasiafi commented Nov 23, 2023 •

edited

Loading

findinpath Nov 26, 2023

findinpath Nov 26, 2023

kasiafi Nov 27, 2023

findinpath Nov 26, 2023

kasiafi Nov 27, 2023

findinpath Nov 26, 2023 •

edited

Loading

kasiafi Nov 27, 2023

mosabua commented Nov 27, 2023

findinpath commented Nov 28, 2023

findepi commented Nov 28, 2023

mosabua commented Nov 28, 2023

	* {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. If we know
	* {@link TimestampWithTimeZoneType}. Such unwrap would not be monotonic. We know

Push down predicate involving timestamp_tz in Delta Lake #19874

Push down predicate involving timestamp_tz in Delta Lake #19874

Conversation

kasiafi commented Nov 23, 2023 • edited Loading

Description

Release notes

findinpath Nov 26, 2023

Choose a reason for hiding this comment

findinpath Nov 26, 2023

Choose a reason for hiding this comment

kasiafi Nov 27, 2023

Choose a reason for hiding this comment

findinpath Nov 26, 2023

Choose a reason for hiding this comment

kasiafi Nov 27, 2023

Choose a reason for hiding this comment

findinpath Nov 26, 2023 • edited Loading

Choose a reason for hiding this comment

kasiafi Nov 27, 2023

Choose a reason for hiding this comment

mosabua commented Nov 27, 2023

findinpath commented Nov 28, 2023

findepi commented Nov 28, 2023

mosabua commented Nov 28, 2023

kasiafi commented Nov 23, 2023 •

edited

Loading

findinpath Nov 26, 2023 •

edited

Loading