-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-26865][SQL] DataSourceV2Strategy should push normalized filters #23770
Conversation
Could you review this, @cloud-fan , @gatorsmile , @hvanhovell , @gengliangwang ? |
good catch! LGTM. |
Thank you for review, @cloud-fan ! |
Test build #102278 has finished for PR 23770 at commit
|
Test build #102279 has finished for PR 23770 at commit
|
retest this please |
@@ -219,6 +219,11 @@ class DataSourceStrategySuite extends PlanTest with SharedSQLContext { | |||
IsNotNull(attrInt))), None) | |||
} | |||
|
|||
test("SPARK-26865 DataSourceV2Strategy should push normalized filters") { | |||
val attrInt = 'cint.int | |||
DataSourceStrategy.normalizeFilters(Seq(IsNotNull(attrInt.withName("CiNt"))), Seq(attrInt)) |
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.
Should we check the result of normalizeFilters
? It seems that the test case will always pass.
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.
+1
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.
Oops. Right. Thanks!
Also, it would be great to have regression test cases like the one in the description of https://issues.apache.org/jira/browse/SPARK-26865 |
Test build #102282 has finished for PR 23770 at commit
|
@gengliangwang . For the second comment, I had the test case at the first commit, but this issue is not about ORC at all, so I changed it. It looks misleading. |
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.
LGTM
Test build #102298 has finished for PR 23770 at commit
|
Retest this please. |
Test build #102305 has finished for PR 23770 at commit
|
Thank you, @cloud-fan , @viirya , @gengliangwang , @HyukjinKwon . |
## What changes were proposed in this pull request? This PR aims to make `DataSourceV2Strategy` normalize filters like [FileSourceStrategy](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategy.scala#L150-L158) when it pushes them into `SupportsPushDownFilters.pushFilters`. ## How was this patch tested? Pass the Jenkins with the newly added test case. Closes apache#23770 from dongjoon-hyun/SPARK-26865. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
## What changes were proposed in this pull request? This PR aims to make `DataSourceV2Strategy` normalize filters like [FileSourceStrategy](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategy.scala#L150-L158) when it pushes them into `SupportsPushDownFilters.pushFilters`. ## How was this patch tested? Pass the Jenkins with the newly added test case. Closes apache#23770 from dongjoon-hyun/SPARK-26865. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
## What changes were proposed in this pull request? This PR aims to make `DataSourceV2Strategy` normalize filters like [FileSourceStrategy](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategy.scala#L150-L158) when it pushes them into `SupportsPushDownFilters.pushFilters`. ## How was this patch tested? Pass the Jenkins with the newly added test case. Closes apache#23770 from dongjoon-hyun/SPARK-26865. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
## What changes were proposed in this pull request? This PR aims to make `DataSourceV2Strategy` normalize filters like [FileSourceStrategy](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/FileSourceStrategy.scala#L150-L158) when it pushes them into `SupportsPushDownFilters.pushFilters`. ## How was this patch tested? Pass the Jenkins with the newly added test case. Closes apache#23770 from dongjoon-hyun/SPARK-26865. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
This PR aims to make
DataSourceV2Strategy
normalize filters like FileSourceStrategy when it pushes them intoSupportsPushDownFilters.pushFilters
.How was this patch tested?
Pass the Jenkins with the newly added test case.