[SPARK-12438][SQL] Add SQLUserDefinedType support for encoder

[viirya]

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

ScalaReflection lacks the support of SQLUserDefinedType. We should add it.

[SparkQA]

Test build #48050 has finished for PR 10390 at commit 51b76b1.

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

[viirya]

cc @cloud-fan @marmbrus @davies

[cloud-fan]

can we also test encoder.fromRow?

we can just create a MyLabelPoint and encode it to InternalRow and decode it back by encoder, and check if the decoded MyLabelPoint is same with the original one.

[viirya]

I added the test now. By doing this, I also found that we need to add UserDefinedType to Cast to make it work.

[cloud-fan]

I think this worth another JIRA.

Is ScalaRelfection the only place that may use UDT in Cast?

[viirya]

I think so, as it is a special use case. I will open another JIRA for it.

[viirya]

This fixing is submitted as pr #10410.

[SparkQA]

Test build #48100 has finished for PR 10390 at commit d01c661.

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

[marmbrus]

I think we need more design work before we implement this. I'm not a huge fan of the current UDT API and I think we will replace it in Spark 2.0.

[viirya]

That is fine. But we will leave it broken until Spark2.0?

[marmbrus]

We can revisit this if we do a 1.7 release, but I don't think that is going to happen.

[viirya]

Personally, I would like to see this to be merged because it blocks my another PR #10283. Can we merge this and go back to modify this part when we revamp UDT API in the future? But of course it depends on you. Thanks.

[marmbrus]

Okay, I'm fine with merging this now to unblock other progress. However, I think we should move the test to be with the other encoder tests.

[viirya]

@marmbrus Thanks. However, to move the test means that we need to move the UDT classes to encoder tests. I think it might duplicate the codes. Is it ok for you?

[SparkQA]

Test build #48468 has finished for PR 10390 at commit 42303d2.

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

[marmbrus]

Do you mean just the example UDT MyLabeledPoint? I'm fine with duplicating that or moving it entirely. If UDTs are defined in catalyst we should also have unit tests for them there.

[viirya]

@marmbrus Not only MyLabeledPoint, which is just a case class. Also including UDT MyDenseVectorUDT and MyDenseVector. I think it might duplicate too many lines of codes from UserDefinedTypeSuite to encoder test suite like ExpressionEncoderSuite.

[cloud-fan]

We alread have UDT in RowEncoderSuite, can we move it to a seperate file, and share it within catalyst tests?

[viirya]

@cloud-fan I just think the same thing. I will try it.

[SparkQA]

Test build #48547 has finished for PR 10390 at commit 72446f1.

• This patch fails to build.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #48548 has finished for PR 10390 at commit 62fa738.

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

[marmbrus]

These changes are not needed.

[viirya]

Forgot to remove them.

[SparkQA]

Test build #48721 has finished for PR 10390 at commit 80a3f7b.

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

[marmbrus]

Thanks, merging to master.