[SPARK-11593][SQL] Replace catalyst converter with RowEncoder in ScalaUDF by viirya · Pull Request #9565 · apache/spark

viirya · 2015-11-09T09:52:49Z

JIRA: https://issues.apache.org/jira/browse/SPARK-11593

We use catalyst converters to transfer catalyst type to and from scala type now. We should use RowEncoder to replace it.

viirya · 2015-11-09T09:56:18Z

Moved here because they should not be included in scalastyle:off section.

Why do we need manually created accessors? All of the arguments to a case class should have public methods already created for them.

Ok. Let me try to use that.

SparkQA · 2015-11-09T10:31:50Z

Test build #45358 has finished for PR 9565 at commit 942dad7.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-09T17:42:34Z

Test build #45382 has finished for PR 9565 at commit 39f6c26.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-09T19:29:24Z

Test build #45390 has finished for PR 9565 at commit 75ffaeb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-11-10T00:36:26Z

retest this please.

SparkQA · 2015-11-10T01:35:34Z

Test build #45449 has finished for PR 9565 at commit 1e13ff9.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-10T04:51:33Z

Test build #45475 has finished for PR 9565 at commit 07ff97a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-11-10T04:52:24Z

cc @davies

davies · 2015-11-10T06:45:33Z

outputEncoder should be created outside of eval, or it will be too slow.

davies · 2015-11-10T06:56:00Z

@viirya Thanks for work on this. I think it's more important to generate the code for converter in generated ScalaUDF.

BTW, the RowEncoder is new in 1.6 (experimental feature), so I'd like to only merge this into master.

viirya · 2015-11-10T06:59:50Z

@davies Thanks for reviewing. I will work on generated version later.

SparkQA · 2015-11-10T12:04:43Z

Test build #45517 has finished for PR 9565 at commit ecf01bf.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2015-11-10T12:36:40Z

retest this please.

SparkQA · 2015-11-10T14:58:21Z

Test build #45520 has finished for PR 9565 at commit ecf01bf.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-11-11T08:01:33Z

Test build #45616 has finished for PR 9565 at commit 39c0b7a.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * sealed abstract class State[S]\n * sealed abstract class StateSpec[KeyType, ValueType, StateType, EmittedType] extends Serializable\n * case class StateSpecImpl[K, V, S, T](\n * sealed abstract class TrackStateDStream[KeyType, ValueType, StateType, EmittedType: ClassTag](\n * class InternalTrackStateDStream[K: ClassTag, V: ClassTag, S: ClassTag, E: ClassTag](\n * case class StateInfo[S](\n * class LimitMarker(val num: Int) extends Serializable\n

viirya · 2015-11-11T08:02:48Z

We use schemaFor to get a catalyst DataType for udf's return type. For Product type, we return a StructType now. That causes a problem in RowEncoder because RowEncoder will try to get a Row not a Product for a field of StructType. You will get a casting exception if your udf returns something like (1, 2).
The problem is a field of StructType in a Row can be a Product or a Row. I modified the getStruct method in Row to turn a Row for a Product.

I think we also need to update the javadoc of Row to say that Product is also a valid value type of StructType.

SparkQA · 2015-11-11T08:38:29Z

Test build #45619 has finished for PR 9565 at commit c910e6e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):\n * sealed abstract class State[S]\n * sealed abstract class StateSpec[KeyType, ValueType, StateType, EmittedType] extends Serializable\n * case class StateSpecImpl[K, V, S, T](\n * sealed abstract class TrackStateDStream[KeyType, ValueType, StateType, EmittedType: ClassTag](\n * class InternalTrackStateDStream[K: ClassTag, V: ClassTag, S: ClassTag, E: ClassTag](\n * case class StateInfo[S](\n * class LimitMarker(val num: Int) extends Serializable\n

viirya · 2016-04-07T10:56:09Z

retest this please.

SparkQA · 2016-04-07T10:58:20Z

Test build #55218 has finished for PR 9565 at commit 597c971.

This patch fails RAT tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2016-04-07T11:19:30Z

retest this please.

SparkQA · 2016-04-07T11:22:33Z

Test build #55219 has started for PR 9565 at commit 597c971.

SparkQA · 2016-04-07T16:24:35Z

Test build #55226 has finished for PR 9565 at commit 405e8b0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2016-04-08T02:49:39Z

retest this please.

Conflicts: project/MimaExcludes.scala

SparkQA · 2016-04-08T03:32:32Z

Test build #55266 has finished for PR 9565 at commit 648c7b2.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-08T04:55:22Z

Test build #55300 has finished for PR 9565 at commit 648c7b2.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

viirya · 2016-04-08T06:12:03Z

retest this please.

SparkQA · 2016-04-08T07:07:47Z

Test build #55301 has finished for PR 9565 at commit 2a0c319.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-08T08:06:59Z

Test build #55325 has finished for PR 9565 at commit 2a0c319.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2016-04-08T08:08:32Z

Finally...tests passed.

viirya · 2016-04-08T08:11:22Z

@davies @rxin This stays here for a while. Recently I re-visit it and fix a previous problem. Can you take a look again? Thanks!

Conflicts: project/MimaExcludes.scala

SparkQA · 2016-04-11T03:30:05Z

Test build #55501 has finished for PR 9565 at commit 884a176.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-11T03:35:39Z

Test build #55502 has finished for PR 9565 at commit 30a867e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2016-04-11T03:59:09Z

hmm. We can't remove non code-generated version of ScalaUDF as any InterpretedProjection with udf will fail...

viirya · 2016-04-13T07:33:20Z

Too replace catalyst converter with RowEncoder in non code-generated ScalaUDF seems not doable due to runtime mirror limitation.

ping @rxin Is it good that I revert the changes of non code-generated ScalaUDF here and just merge code-generated version?

viirya · 2016-04-19T06:29:43Z

Close this now. Maybe revisit this in the future.

rxin · 2016-04-19T06:42:16Z

What's the problem with runtime mirror?

viirya · 2016-04-19T06:54:19Z

When using member method as udf., for example, def createTransformFunc in org.apache.spark.ml.Transformer, jenkins tests always get an exception. Looks like it can't recognize the type of this kind method.

Otherwise, it works well.

BTW, I can't reproduce that exception locally. Maybe java version matters.

koertkuipers · 2017-02-04T17:47:51Z

i think this would be very helpful. the difference in behaviour of scala udfs and scala functions used in dataset transformations is a constant source of confusion for my users.

for example the lack of support for Option to declare nullable input types, and the need to use untyped Row objects in UDFs for structs are inconsistent with how things are done when Encoders are used.

Replace catalyst converter with RowEncoder.
942dad7

viirya reviewed Nov 9, 2015
View reviewed changes

Add UserDefinedType to RowEncoder.
39f6c26

Fix scala style.
75ffaeb

Call serialize on udt instead of user class.
1e13ff9

Add getField for UserDefinedType.
07ff97a

davies reviewed Nov 10, 2015
View reviewed changes

Move outputEncoder outside of eval and add calling copy().
ecf01bf

viirya added 2 commits November 11, 2015 15:47

Merge remote-tracking branch 'upstream/master' into rowencoder-scalaudf
5186777

Replace catalyst converter with RowEncoder for the generated ScalaUDF.
39c0b7a

viirya reviewed Nov 11, 2015
View reviewed changes

Fix scala style.
c910e6e

Fix passing null into ScalaUDF.
405e8b0

Check PrimitiveType.
648c7b2

viirya added 2 commits April 8, 2016 02:51

Merge remote-tracking branch 'upstream/master' into rowencoder-scalaudf
10a9f91

Merge remote-tracking branch 'upstream/master' into rowencoder-scalaudf
2a0c319
Conflicts: project/MimaExcludes.scala

viirya added 3 commits April 11, 2016 02:46

Merge remote-tracking branch 'upstream/master' into rowencoder-scalaudf
21a2af2
Conflicts: project/MimaExcludes.scala

Remove ScalaUDF non code-generated evaluation support.
884a176

Remove unnecessary import.
30a867e

viirya closed this Apr 19, 2016

viirya deleted the rowencoder-scalaudf branch December 27, 2023 18:33

Uh oh!

Conversation

viirya commented Nov 9, 2015

Uh oh!

viiryaNov 9, 2015

Choose a reason for hiding this comment

Uh oh!

marmbrusDec 1, 2015

Choose a reason for hiding this comment

Uh oh!

viiryaDec 1, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 9, 2015

Uh oh!

SparkQA commented Nov 9, 2015

Uh oh!

SparkQA commented Nov 9, 2015

Uh oh!

viirya commented Nov 10, 2015

Uh oh!

SparkQA commented Nov 10, 2015

Uh oh!

SparkQA commented Nov 10, 2015

Uh oh!

viirya commented Nov 10, 2015

Uh oh!

daviesNov 10, 2015

Choose a reason for hiding this comment

Uh oh!

davies commented Nov 10, 2015

Uh oh!

viirya commented Nov 10, 2015

Uh oh!

SparkQA commented Nov 10, 2015

Uh oh!

viirya commented Nov 10, 2015

Uh oh!

SparkQA commented Nov 10, 2015

Uh oh!

SparkQA commented Nov 11, 2015

Uh oh!

viiryaNov 11, 2015

Choose a reason for hiding this comment

Uh oh!

cloud-fanNov 14, 2015

Choose a reason for hiding this comment

Uh oh!

viiryaNov 14, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 11, 2015

Uh oh!

viirya commented Apr 7, 2016

Uh oh!

SparkQA commented Apr 7, 2016

Uh oh!

viirya commented Apr 7, 2016

Uh oh!

SparkQA commented Apr 7, 2016

Uh oh!

SparkQA commented Apr 7, 2016

Uh oh!

viirya commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

viirya commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 8, 2016

Uh oh!

viirya commented Apr 8, 2016

Uh oh!

viirya commented Apr 8, 2016

Uh oh!

SparkQA commented Apr 11, 2016

viirya commented Apr 19, 2016•
edited
Loading