-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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.
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
Refactor dynamic filtering configs #5262
Conversation
f36ee6b
to
2403f34
Compare
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/sql/planner/LocalExecutionPlanner.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/sql/planner/LocalExecutionPlanner.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
856244e
to
f5e09e0
Compare
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-hive/src/test/java/io/prestosql/plugin/hive/TestHiveDynamicPartitionPruning.java
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/sql/planner/LocalExecutionPlanner.java
Outdated
Show resolved
Hide resolved
presto-main/src/test/java/io/prestosql/operator/BenchmarkDynamicFilterSourceOperator.java
Outdated
Show resolved
Hide resolved
presto-main/src/test/java/io/prestosql/operator/BenchmarkDynamicFilterSourceOperator.java
Outdated
Show resolved
Hide resolved
f3b5974
to
c364f28
Compare
|
Results for some more permutations
|
c364f28
to
7e280fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comments
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Show resolved
Hide resolved
presto-memory/src/test/java/io/prestosql/plugin/memory/TestMemorySmoke.java
Show resolved
Hide resolved
7e280fd
to
ae410c3
Compare
ae410c3
to
9e80589
Compare
@@ -166,7 +170,7 @@ public void testJoinWithNonSelectiveBuildSide() | |||
} | |||
|
|||
@Test(timeOut = 30_000) | |||
public void testJoinLargeBuildSideNoDynamicFiltering() | |||
public void testJoinLargeBuildSideDynamicFiltering() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call it testJoinLargeBuildSideRangeDynamicFiltering
Also, we are missing test case for "too" large build side for DFs to be collected. It would be good to restore it, but it's not critical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, I'll add the too-large-no-df test as well in a different PR
presto-hive/src/test/java/io/prestosql/plugin/hive/TestHiveDynamicPartitionPruning.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/io/prestosql/execution/DynamicFilterConfig.java
Outdated
Show resolved
Hide resolved
presto-main/src/test/java/io/prestosql/operator/BenchmarkDynamicFilterSourceOperator.java
Outdated
Show resolved
Hide resolved
Moved dynamic filtering related configs to DynamicFilterConfig. Added enable-large-dynamic-filters to allow collection of large dynamic filters. Removed existing configs dynamic-filtering-max-per-driver-row-count, dynamic-filtering-max-per-driver-size, dynamic-filtering-range-row-limit-per-driver and their session properties. These are replaced by new properties based on join distribution type and value of enable_large_dynamic_filters.
9e80589
to
a745094
Compare
merged thanks! |
Moved dynamic filtering related configs to DynamicFilterConfig.
Added enable-large-dynamic-filters to allow collection of
large dynamic filters.
Removed existing configs dynamic-filtering-max-per-driver-row-count,
dynamic-filtering-max-per-driver-size,
dynamic-filtering-range-row-limit-per-driver and their session
properties. These are replaced by new properties based on
join distribution type and value of enable_large_dynamic_filters.