[SPARK-21396][SQL] Fixes MatchError when UDTs are passed through Hive Thriftserver

[atallahhezbor]

Signed-off-by: Atallah Hezbor [email protected]

What changes were proposed in this pull request?

This PR proposes modifying the match statement that gets the columns of a row in HiveThriftServer. There was previously no case for UserDefinedType, so querying a table that contained them would throw a match error. The changes catch that case and return the string representation.

How was this patch tested?

While I would have liked to add a unit test, I couldn't easily incorporate UDTs into the HiveThriftServer2Suites pipeline. With some guidance I would be happy to push a commit with tests.

Instead I did a manual test by loading a DataFrame with Point UDT in a spark shell with a HiveThriftServer. Then in beeline, connecting to the server and querying that table.

Here is the result before the change

0: jdbc:hive2://localhost:10000> select * from chicago;
Error: scala.MatchError: org.apache.spark.sql.PointUDT@2d980dc3 (of class org.apache.spark.sql.PointUDT) (state=,code=0)

And after the change:

0: jdbc:hive2://localhost:10000> select * from chicago;
+---------------------------------------+--------------+------------------------+---------------------+--+
|                __fid__                | case_number  |          dtg           |        geom         |
+---------------------------------------+--------------+------------------------+---------------------+--+
| 109602f9-54f8-414b-8c6f-42b1a337643e  | 2            | 2016-01-01 19:00:00.0  | POINT (-77 38)      |
| 709602f9-fcff-4429-8027-55649b6fd7ed  | 1            | 2015-12-31 19:00:00.0  | POINT (-76.5 38.5)  |
| 009602f9-fcb5-45b1-a867-eb8ba10cab40  | 3            | 2016-01-02 19:00:00.0  | POINT (-78 39)      |
+---------------------------------------+--------------+------------------------+---------------------+--+

[gatorsmile]

cc @liufengdb

[felixcheung]

Jenkins test this please

[SparkQA]

Test build #86597 has finished for PR 20385 at commit c8fb436.

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

[liufengdb]

It is possible from.get(ordinal) returns null, then a null pointer exception. I think a better way to add this case is by the method HiveUtils.toHiveString, which can potentially be reused and tested.

[liufengdb]

Looks like a test is necessary.

[gatorsmile]

The unit test cases are needed for HiveUtils.toHiveString.

[atallahhezbor]

@liufengdb @gatorsmile I'm happy to write a unit test if you require it. Though as I mentioned before, I did not see a clear way of testing the functionality in SparkExecuteStatementOperation. It looks to me that a lot of those functions are not tested directly.

Also @gatorsmile, I'm not sure I understand you. If I move the functionality to HiveUtils.toHiveString, are you saying I should write a unit test of that whole function?

[gatorsmile]

@atallahhezbor Yeah! Please help us improve the test coverage. We do not have a clear way to test the functionality in SparkExecuteStatementOperation

Adding unit test cases for HiveUtils.toHiveString is enough if we move the code changes to HiveUtils.toHiveString

[gatorsmile]

ok to test

[liufengdb]

Actually, one more thing, do you need to consider the UDT as one attribute of a structured type? https://github.com/apache/spark/pull/20385/files#diff-842e3447fc453de26c706db1cac8f2c4L467

[SparkQA]

Test build #86887 has finished for PR 20385 at commit e05041f.

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

[gatorsmile]

LGTM

@atallahhezbor Could you submit another PR to address the comment from @liufengdb ?

This fix is nice to have in Spark 2.3. Let merge this now.

Thanks! Merged to master/2.3