[SPARK-29518][SQL][TEST] Benchmark date_part for INTERVAL
#26175
Conversation
|
Test build #112316 has finished for PR 26175 at commit
|
| case "extract" => s"EXTRACT($field FROM ${castExpr(from)})" | ||
| case "date_part" => s"DATE_PART('$field', ${castExpr(from)})" | ||
| case "extract" => s"EXTRACT($field FROM ${castExpr(from)}) AS $field" | ||
| case "date_part" => s"DATE_PART('$field', ${castExpr(from)}) AS $field" |
There was a problem hiding this comment.
Out of curiosity, why do we need to add alias?
There was a problem hiding this comment.
When I debugged this, I showed dataframes to terminal. And printed tables were so wide.
| "QUARTER", "MONTH", "DAY", | ||
| "HOUR", "MINUTE", "SECOND", | ||
| "MILLISECONDS", "MICROSECONDS", "EPOCH") | ||
| val settings = Map( |
There was a problem hiding this comment.
not a big deal but Settings seems only used within runBenchmarkSuite. I think it's fine to just make this pretty with indentation.
There was a problem hiding this comment.
? Could you clarify, please. Do you want to replace Settings by let's say tuples?
There was a problem hiding this comment.
This one?
for {
(dataType, (fields, funcs, iterNum)) <- Map(
"timestamp" -> (datetimeFields, Seq("extract", "date_part"), N),
"date" -> (datetimeFields, Seq("extract", "date_part"), N),
"interval" -> (intervalFields, Seq("date_part"), N))
func <- funcs} {There was a problem hiding this comment.
yup, I was thinking like that. I don't mind if you prefer the current way. no biggie.
|
Merged to master. |
4 participants
What changes were proposed in this pull request?
I extended
ExtractBenchmarkto support theINTERVALtype of thesourceparameter of thedate_partfunction.Why are the changes needed?
date_partfunction in the future.date_partfor theINTERVALtypeDoes this PR introduce any user-facing change?
No
How was this patch tested?
By running the benchmark and print out produced values per each
fieldvalue.