Skip to content
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

Jump to bottom

Remove unsupported dynamic filters from query plan #2363

Merged
merged 2 commits into from
Jan 3, 2020
Merged

Remove unsupported dynamic filters from query plan #2363

merged 2 commits into from
Jan 3, 2020

Conversation

raunaqmorarka
Copy link
Member

Fixes #2355

@cla-bot cla-bot bot added the cla-signed label Dec 30, 2019
sopel39
sopel39 reviewed Dec 30, 2019
View reviewed changes
presto-main/src/main/java/io/prestosql/sql/DynamicFilters.java Outdated Show resolved Hide resolved
presto-main/src/main/java/io/prestosql/sql/DynamicFilters.java Outdated Show resolved Hide resolved
presto-main/src/main/java/io/prestosql/sql/DynamicFilters.java Outdated Show resolved Hide resolved
...n/src/main/java/io/prestosql/sql/planner/iterative/rule/RemoveUnsupportedDynamicFilters.java Outdated Show resolved Hide resolved
...n/src/main/java/io/prestosql/sql/planner/iterative/rule/RemoveUnsupportedDynamicFilters.java Outdated Show resolved Hide resolved
...n/src/main/java/io/prestosql/sql/planner/iterative/rule/RemoveUnsupportedDynamicFilters.java Outdated Show resolved Hide resolved
presto-main/src/test/java/io/prestosql/sql/planner/TestDynamicFilter.java Outdated Show resolved Hide resolved
presto-main/src/test/java/io/prestosql/sql/planner/TestDynamicFilter.java Outdated Show resolved Hide resolved
@raunaqmorarka raunaqmorarka force-pushed the unsupported_df branch 3 times, most recently from d86216f to d139fc7 Compare December 30, 2019 19:44
sopel39
sopel39 reviewed Dec 31, 2019
View reviewed changes
Copy link
Member

@sopel39 sopel39 left a comment

Choose a reason for hiding this comment

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

minor comments

presto-main/src/main/java/io/prestosql/sql/planner/sanity/DynamicFiltersChecker.java Show resolved Hide resolved
...n/src/main/java/io/prestosql/sql/planner/iterative/rule/RemoveUnsupportedDynamicFilters.java Outdated Show resolved Hide resolved
...n/src/main/java/io/prestosql/sql/planner/iterative/rule/RemoveUnsupportedDynamicFilters.java Outdated Show resolved Hide resolved
presto-main/src/test/java/io/prestosql/sql/planner/TestDynamicFilter.java Outdated Show resolved Hide resolved
@raunaqmorarka raunaqmorarka force-pushed the unsupported_df branch 2 times, most recently from d64ae27 to cd0d78a Compare January 1, 2020 20:46
@raunaqmorarka raunaqmorarka requested a review from sopel39 January 1, 2020 20:50
sopel39
sopel39 previously approved these changes Jan 2, 2020
View reviewed changes
Copy link
Member

@sopel39 sopel39 left a comment

Choose a reason for hiding this comment

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

lgtm % https://github.com/prestosql/presto/pull/2363/files#r362462870

@sopel39
Copy link
Member

sopel39 commented Jan 2, 2020

Failed with (#2390): https://pipelines.actions.githubusercontent.com/sJrKZtZsjYLeyqPixYxcTJp8fX9YNlwyJbmEGSC0oymXVDSpbK/_apis/pipelines/1/runs/1450/signedlogcontent/31?urlExpires=2020-01-02T14%3A52%3A10.2465689Z&urlSigningMethod=HMACV1&urlSignature=PS2J4QbJgqFto8UqUJBFOVwbFvhquB9cH0HYJd8q22c%3D

@sopel39 sopel39 dismissed their stale review January 2, 2020 15:31

there is still one issue

@raunaqmorarka raunaqmorarka requested a review from sopel39 January 2, 2020 20:29
@sopel39 sopel39 merged commit dba0d78 into trinodb:master Jan 3, 2020
@sopel39
Copy link
Member

sopel39 commented Jan 3, 2020

merged, thanks!

@sopel39 sopel39 mentioned this pull request Jan 3, 2020
Closed
6 tasks
@raunaqmorarka raunaqmorarka deleted the unsupported_df branch January 3, 2020 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sopel39 sopel39 sopel39 approved these changes

Assignees
No one assigned
Labels
cla-signed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unsupported dynamic filter present in query plan
2 participants
@raunaqmorarka @sopel39