Optional check for query partition filter for Delta #18345

marcinsbd · 2023-07-19T12:13:01Z

Description

For Delta partitioned tables, we should reject the table scan produced by the planner when the query does not have partition field.

Additional context and related issues

Release notes

( ) This is not user-visible or 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:

# DeltaLake
* Add option to enforce that a filter on a partition key be present in the query. This can be enabled by setting the 
  ``delta.query-partition-filter-required`` config property or the ``query_partition_filter_required`` session property 
  to ``true``.

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

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.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/TestDeltaLakeConnectorTest.java

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeRequireQueryPartitionsFilter.java

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

ebyhr · 2023-07-21T09:22:59Z

cc: @findepi @findinpath

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

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

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

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

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

docs/src/main/sphinx/connector/delta-lake.rst

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

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

Praveen2112

% changes

Praveen2112 · 2023-08-01T13:29:08Z

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

+            List<String> partitionColumns = deltaLakeTableHandle.getMetadataEntry().getCanonicalPartitionColumns();
+            if (!partitionColumns.isEmpty()) {
+                if (deltaLakeTableHandle.getAnalyzeHandle().isPresent()) {
+                    throw new TrinoException(QUERY_REJECTED, "The session with enabled property delta.query-partition-filter-required does not work with ANALYZE statement.");


Can we re-word the error as Session with delta.query-partition-filter-required enabled does not work with ANALYZE statement

WDYT @findinpath

delta.query-partition-filter-required is not a session option, but rather a configuration option.

cc @marcinsbd

nevertheless, @Praveen2112 , @findinpath , is the exception message OK? if not pls provide how it's supposed be formulated

My take on the exception message:

Suggested change

throw new TrinoException(QUERY_REJECTED, "The session with enabled property delta.query-partition-filter-required does not work with ANALYZE statement.");

throw new TrinoException(QUERY_REJECTED, "ANALYZE statement can't be performed on partitioned tables because filtering is required on at least one partition. The partition filtering check can be however disabled with the catalog session property 'query_partition_filter_required'.");

Consider that the analysts interacting with Trino don't necessarily have access to the configuration properties (but they do have access to the session properties).

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

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

findinpath

LGTM % comments

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

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeRequireQueryPartitionsFilter.java

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

Praveen2112 · 2023-08-02T13:06:17Z

Thanks for working on this.

mosabua · 2023-08-02T16:40:29Z

Does this really not need a release notes entry?

Praveen2112 · 2023-08-03T06:24:37Z

@mosabua Updated the release notes

findepi · 2023-08-03T13:34:12Z

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

+                if (deltaLakeTableHandle.getAnalyzeHandle().isPresent()) {
+                    throw new TrinoException(
+                            QUERY_REJECTED,
+                            "ANALYZE statement can not be performed on partitioned tables because filtering is required on at least one partition. However, the partition filtering check can be disabled with the catalog session property 'query_partition_filter_required'.");


Add a code comment that Delta ANAlyze currently does not support specificying partitioning predicates (unlike eg Hive's)

cla-bot bot added the cla-signed label Jul 19, 2023

marcinsbd requested review from krvikash, Praveen2112, atanasenko, vlad-lyutenko and kokosing July 19, 2023 12:36

github-actions bot added tests:hive delta-lake Delta Lake connector labels Jul 19, 2023

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 4dfc3f3 to 5eb28fd Compare July 19, 2023 13:51

Praveen2112 reviewed Jul 19, 2023

View reviewed changes

krvikash reviewed Jul 20, 2023

View reviewed changes

vlad-lyutenko reviewed Jul 20, 2023

View reviewed changes

...rino-delta-lake/src/test/java/io/trino/plugin/deltalake/BaseDeltaLakeConnectorSmokeTest.java Outdated Show resolved Hide resolved

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch 2 times, most recently from 998119d to 914337f Compare July 21, 2023 08:36

marcinsbd marked this pull request as draft July 21, 2023 09:19

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch 3 times, most recently from 2d099ed to 4d08f8a Compare July 24, 2023 11:01

marcinsbd requested review from Praveen2112, vlad-lyutenko, krvikash, findepi and ebyhr July 25, 2023 08:22

marcinsbd marked this pull request as ready for review July 25, 2023 08:23

Praveen2112 reviewed Jul 25, 2023

View reviewed changes

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 4d08f8a to 5a11a61 Compare July 26, 2023 09:46

marcinsbd requested review from Praveen2112 and alexjo2144 July 26, 2023 09:47

Praveen2112 reviewed Jul 27, 2023

View reviewed changes

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java Outdated Show resolved Hide resolved

findinpath reviewed Aug 1, 2023

View reviewed changes

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeTableHandle.java Show resolved Hide resolved

findinpath reviewed Aug 1, 2023

View reviewed changes

Add missing fields when calling constructor

d1c324c

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 4d21724 to 9d33b9a Compare August 1, 2023 12:49

marcinsbd requested a review from findinpath August 1, 2023 12:51

Praveen2112 approved these changes Aug 1, 2023

View reviewed changes

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 9d33b9a to 64bc8fc Compare August 1, 2023 17:58

marcinsbd requested a review from Praveen2112 August 1, 2023 17:59

findinpath reviewed Aug 1, 2023

View reviewed changes

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 64bc8fc to 49a4cf1 Compare August 1, 2023 21:41

ebyhr reviewed Aug 1, 2023

View reviewed changes

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 49a4cf1 to 12c6518 Compare August 2, 2023 00:00

marcinsbd requested review from ebyhr and findinpath August 2, 2023 00:02

findinpath approved these changes Aug 2, 2023

View reviewed changes

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from 12c6518 to d0e750b Compare August 2, 2023 08:08

marcinsbd requested a review from findinpath August 2, 2023 08:08

findinpath approved these changes Aug 2, 2023

View reviewed changes

Praveen2112 approved these changes Aug 2, 2023

View reviewed changes

marcinsbd mentioned this pull request Aug 2, 2023

Enforce delta.query-partition-filter-required property in Delta Lake table_changes table function #18498

Open

Optional check for query partition filter for Delta

5b2bb16

marcinsbd force-pushed the enforce_filter_on_partitioned_columnn_in_deltalake branch from d0e750b to 5b2bb16 Compare August 2, 2023 11:01

Praveen2112 merged commit 1ba82f8 into trinodb:master Aug 2, 2023

github-actions bot added this to the 423 milestone Aug 2, 2023

mosabua mentioned this pull request Aug 2, 2023

Add Trino 423 release notes #18496

Merged

findepi reviewed Aug 3, 2023

View reviewed changes

krvikash mentioned this pull request Nov 27, 2023

Optional check for query partition filter for Hudi #19906

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional check for query partition filter for Delta #18345

Optional check for query partition filter for Delta #18345

marcinsbd commented Jul 19, 2023 •

edited by Praveen2112

Loading

ebyhr commented Jul 21, 2023

Praveen2112 left a comment

Praveen2112 Aug 1, 2023

findinpath Aug 1, 2023

marcinsbd Aug 1, 2023

findinpath Aug 1, 2023 •

edited

Loading

findinpath left a comment

Praveen2112 commented Aug 2, 2023

mosabua commented Aug 2, 2023

Praveen2112 commented Aug 3, 2023

findepi Aug 3, 2023

	throw new TrinoException(QUERY_REJECTED, "The session with enabled property delta.query-partition-filter-required does not work with ANALYZE statement.");
	throw new TrinoException(QUERY_REJECTED, "ANALYZE statement can't be performed on partitioned tables because filtering is required on at least one partition. The partition filtering check can be however disabled with the catalog session property 'query_partition_filter_required'.");

Optional check for query partition filter for Delta #18345

Optional check for query partition filter for Delta #18345

Conversation

marcinsbd commented Jul 19, 2023 • edited by Praveen2112 Loading

Description

Additional context and related issues

Release notes

ebyhr commented Jul 21, 2023

Praveen2112 left a comment

Choose a reason for hiding this comment

Praveen2112 Aug 1, 2023

Choose a reason for hiding this comment

findinpath Aug 1, 2023

Choose a reason for hiding this comment

marcinsbd Aug 1, 2023

Choose a reason for hiding this comment

findinpath Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

findinpath left a comment

Choose a reason for hiding this comment

Praveen2112 commented Aug 2, 2023

mosabua commented Aug 2, 2023

Praveen2112 commented Aug 3, 2023

findepi Aug 3, 2023

Choose a reason for hiding this comment

marcinsbd commented Jul 19, 2023 •

edited by Praveen2112

Loading

findinpath Aug 1, 2023 •

edited

Loading