-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-31489][SQL] Fix pushing down filters with java.time.LocalDate values in ORC
#28261
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
Conversation
|
Test build #121482 has finished for PR 28261 at commit
|
|
jenkins, retest this, please |
|
Test build #121485 has finished for PR 28261 at commit
|
|
@dongjoon-hyun @cloud-fan @HyukjinKwon Could you take a look at the PR, please. |
| val dates = Seq("2017-08-18", "2017-08-19", "2017-08-20", "2017-08-21").map { day => | ||
| LocalDate.parse(day) | ||
| } | ||
| withSQLConf(SQLConf.DATETIME_JAVA8API_ENABLED.key -> "true") { |
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.
It makes me realize that this config can break data source v1/v2 as it changes the value in Filter.
Shall we fix DataSourceAnalysis.translateLeafNodeFilter instead to always return java 7 Date/Timestamp?
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.
@cloud-fan Here is the PR #28272
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.
@dongjoon-hyun ^^ The PR covers this PR
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.
Got it. Thanks, @MaxGekk !
|
Thank you for pinging me, @MaxGekk .
|
java.time.LocalDate values in ORCjava.time.LocalDate values in ORC
|
Retest this please. |
| } | ||
| } | ||
|
|
||
| test("filter pushdown - local date") { |
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.
nit. test("SPARK-31489: filter.
|
For this PR, please pass the PR builders with both Hive 1.2 and Hive 2.3. |
|
Test build #121523 has finished for PR 28261 at commit
|
|
jenkins, retest this, please |
java.time.LocalDate values in ORCjava.time.LocalDate values in ORC
|
Test build #121696 has finished for PR 28261 at commit
|
|
Test build #121685 has finished for PR 28261 at commit
|
|
retest this please |
|
Test build #121719 has finished for PR 28261 at commit
|
|
jenkins, retest this, please |
|
Test build #121735 has finished for PR 28261 at commit
|
|
jenkins, retest this, please |
|
Test build #121754 has finished for PR 28261 at commit
|
java.time.LocalDate values in ORCjava.time.LocalDate values in ORC
|
jenkins, retest this, please |
|
Test build #121813 has finished for PR 28261 at commit
|
|
jenkins, retest this, please |
|
Test build #121833 has finished for PR 28261 at commit
|
|
Test build #121836 has finished for PR 28261 at commit
|
java.time.LocalDate values in ORCjava.time.LocalDate values in ORC
|
jenkins, retest this, please |
|
Test build #121847 has finished for PR 28261 at commit
|
|
This PR has been tested for hive-1.2 and hive-2.3, and it is ready for review. @cloud-fan @dongjoon-hyun Please, take a look at it. |
dongjoon-hyun
left a comment
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, LGTM. Thank you, @MaxGekk and @cloud-fan .
Merged to master/3.0.
…` values in ORC ### What changes were proposed in this pull request? Convert `java.time.LocalDate` to `java.sql.Date` in pushed down filters to ORC datasource when Java 8 time API enabled. Closes #28272 ### Why are the changes needed? The changes fix the exception raised while pushing date filters when `spark.sql.datetime.java8API.enabled` is set to `true`: ``` Wrong value class java.time.LocalDate for DATE.EQUALS leaf java.lang.IllegalArgumentException: Wrong value class java.time.LocalDate for DATE.EQUALS leaf at org.apache.hadoop.hive.ql.io.sarg.SearchArgumentImpl$PredicateLeafImpl.checkLiteralType(SearchArgumentImpl.java:192) at org.apache.hadoop.hive.ql.io.sarg.SearchArgumentImpl$PredicateLeafImpl.<init>(SearchArgumentImpl.java:75) at org.apache.hadoop.hive.ql.io.sarg.SearchArgumentImpl$BuilderImpl.equals(SearchArgumentImpl.java:352) at org.apache.spark.sql.execution.datasources.orc.OrcFilters$.buildLeafSearchArgument(OrcFilters.scala:229) ``` ### Does this PR introduce any user-facing change? Yes ### How was this patch tested? Added tests to `OrcFilterSuite`. Closes #28261 from MaxGekk/orc-date-filter-pushdown. Authored-by: Max Gekk <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit bd139bd) Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
Convert
java.time.LocalDatetojava.sql.Datein pushed down filters to ORC datasource when Java 8 time API enabled.Closes #28272
Why are the changes needed?
The changes fix the exception raised while pushing date filters when
spark.sql.datetime.java8API.enabledis set totrue:Does this PR introduce any user-facing change?
Yes
How was this patch tested?
Added tests to
OrcFilterSuite.