[SPARK-19254][SQL] Support Seq, Map, and Struct in functions.lit #16610
Conversation
|
Since spark possibly supports comparable |
|
Test build #71473 has finished for PR 16610 at commit
|
|
@hvanhovell Could you give me some insights? |
| * @group normal_funcs | ||
| * @since 2.2.0 | ||
| */ | ||
| def lit2[T : TypeTag](literal: T): Column = { |
There was a problem hiding this comment.
Do you think there is a way we can actually avoid this?
If we must why not name it typedLit?
cc @cloud-fan
There was a problem hiding this comment.
I just named this without strong thoughts, so typedLit looks good to me.
| @@ -153,6 +154,12 @@ object Literal { | |||
| Literal(CatalystTypeConverters.convertToCatalyst(v), dataType) | |||
| } | |||
|
|
|||
| def create[T : TypeTag](v: T): Literal = { | |||
| val ScalaReflection.Schema(dataType, _) = ScalaReflection.schemaFor[T] | |||
| val convert = CatalystTypeConverters.createToCatalystConverter(dataType) | |||
There was a problem hiding this comment.
Call create(v, dataType) instead?
There was a problem hiding this comment.
Aha, you're right. I'll fix this.
There was a problem hiding this comment.
Oh, my bad. I found the code you suggested made some tests fail. Actually, CatalystTypeConverters.convertToCatalyst does not do the same thing with CatalystTypeConverters.createToCatalystConverter(dataType). For instance, CatalystTypeConverters.convertToCatalyst does not convert TupleX to GenericInternalRow.
There was a problem hiding this comment.
Shouldn't you be going to talks :)
There was a problem hiding this comment.
haha, actually I'm joining the dev talk now ;)
|
Test build #72652 has finished for PR 16610 at commit
|
|
Test build #72659 has finished for PR 16610 at commit
|
|
ping |
| * @group normal_funcs | ||
| * @since 2.2.0 | ||
| */ | ||
| def typedLit[T : TypeTag](literal: T): Column = { |
| * @since 2.2.0 | ||
| */ | ||
| def typedLit[T : TypeTag](literal: T): Column = { | ||
| literal match { |
There was a problem hiding this comment.
This match statement is slightly hair raising (I know this is copied from lit(..)), how about:
literal match {
case c: Column => c
case s: Symbol => new ColumnName(s.name)
case _ => Column(Literal.create(literal))
}You could also consider mapping the untyped lit(..) function to this function.
There was a problem hiding this comment.
@hvanhovell How about the fix? 4f65fe2
|
Test build #73161 has finished for PR 16610 at commit
|
|
Test build #73162 has finished for PR 16610 at commit
|
|
Test build #73165 has finished for PR 16610 at commit
|
|
ping |
| val literalExpr = Literal(literal) | ||
| Column(literalExpr) | ||
| /** | ||
| * Creates a [[Column]] of literal value. |
There was a problem hiding this comment.
We should explain when you want to use this method instead of lit.
| * @group normal_funcs | ||
| * @since 2.2.0 | ||
| */ | ||
| def typedLit[T : TypeTag](literal: T): Column = literal match { |
There was a problem hiding this comment.
cc @cloud-fan wdyt about the naming?
| @@ -99,6 +99,8 @@ object functions { | |||
| * The passed in object is returned directly if it is already a [[Column]]. | |||
| * If the object is a Scala Symbol, it is converted into a [[Column]] also. | |||
| * Otherwise, a new [[Column]] is created to represent the literal value. | |||
| * Different from [[lit]], this functions can handle all the types in Scala, | |||
There was a problem hiding this comment.
The difference between this function and [[lit]] is that this function can handle parameterized scala types e.g.: List, Seq and Map.
|
Test build #73752 has finished for PR 16610 at commit
|
|
Test build #73757 has finished for PR 16610 at commit
|
|
LGTM - merging to master. Thanks! |
What changes were proposed in this pull request?
This pr is to support Seq, Map, and Struct in functions.lit; it adds a new IF named
lit2withTypeTagfor avoiding type erasure.How was this patch tested?
Added tests in
LiteralExpressionSuite