[SPARK-16731][SQL] use StructType in CatalogTable and remove CatalogColumn

[cloud-fan]

What changes were proposed in this pull request?

StructField has very similar semantic with CatalogColumn, except that CatalogColumn use string to express data type. I think it's reasonable to use StructType as the CatalogTable.schema and remove CatalogColumn.

How was this patch tested?

existing tests.

[cloud-fan]

cc @yhuai @liancheng

[SparkQA]

Test build #62873 has finished for PR 14363 at commit 776b267.

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

[SparkQA]

Test build #62879 has finished for PR 14363 at commit 3f5480c.

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

[SparkQA]

Test build #62889 has finished for PR 14363 at commit addc585.

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

[SparkQA]

Test build #62965 has finished for PR 14363 at commit 97b0492.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • implicit class SchemaAttribute(f: StructField)

[SparkQA]

Test build #62969 has finished for PR 14363 at commit b781ef8.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • implicit class SchemaAttribute(f: StructField)

[gatorsmile]

This is the change we have to make if we convert CatalogColumn to StructField. Previously, we do the data type parsing only when we need to use it. Here, we parse it when we read from the Hive Catalog. That means, it could break the behaviors in some extreme cases. For example, it sounds like hc.getType could return null? or Hive could return some data types we might not recognize. We could hit the exception from Parser, right?

That means, the caller of fromHiveColumn will also get the exception. getTableOption is the caller. I am just wondering if we do not want to see this kind of exception when doing getTableOption. Or maybe issue a nicer error message here?

[cloud-fan]

So the behaviour change is: previously if a hive table contains type string that we can't parse, we are still able to describe it, but throw an exception if we try to read it. After this PR, we will throw an exception when we try to read its table meta from hive meta store.

I think it's ok to break it, but need better error message. what do you think? cc @yhuai @liancheng

[SparkQA]

Test build #63045 has finished for PR 14363 at commit 80d2f50.

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

[yhuai]

Do we know which hive type strings cannot be parsed by spark?

[yhuai]

LGTM. Thanks. Merging to master.

[cloud-fan]

Do we know which hive type strings cannot be parsed by spark?

varchar(length) and char(length). see #14363 (comment) for what we break.

[yhuai]

Thanks. But, what are specific cases are not supported? If there is any case, we should make change to support that, right?

[cloud-fan]

for a hive table(created by hive) with varchar(length) column, we can describe it but can't read data from it before this PR. Now we can't describe it. Do you think we should fix it? BTW there is no test for this case.

[gatorsmile]

TestHive.sessionState.metadataHive.runSqlHive("CREATE TABLE test (id varchar(50))")
TestHive.sessionState.metadataHive.runSqlHive("INSERT INTO TABLE test VALUES ('4')")
spark.sql("select * from test").show()
spark.sql("describe test").show()

Are you saying this case? I tried. It works.

[cloud-fan]

Oh sorry I misread our parser rules. varchar(length) is supported but the length is ignored. I checked with hive again, looks like the only unsupported data type is UNIONTYPE

[gatorsmile]

Your concern is valid. We are missing the test cases for verifying these scenarios.

I saw a discussion in a wechat group about the issue in integration between Hive and Spark. They are complaining Spark is unable to read the data wrote by Hive. In Hive refactoring, I am wondering if we also need to build the test cases to cover these cases?

[lianhuiwang]

@cloud-fan There is a case that i met. The varchar(length)/char(length) type is not a String Type. But now SparkSQL consider them a string type. So there are different result with the following example:
TestHive.sessionState.metadataHive.runSqlHive("CREATE TABLE test (id varchar(50))")
TestHive.sessionState.metadataHive.runSqlHive("INSERT INTO TABLE test VALUES ('abcdef')")
TestHive.sessionState.metadataHive.runSqlHive("CREATE TABLE test_parquet (id varchar(2) stored as parquet)")
TestHive.sessionState.metadataHive.runSqlHive("insert overwrite table varchar_parquet1 select * from test")
the result of varchar_parquet1 are 'ab'.
spark.sql("insert overwrite table varchar_parquet1 select * from test").show()
the result of varchar_parquet1 are 'abcdef'.

[cloud-fan]

Well, Spark SQL is not announced to be fully compatible with hive, I think it's reasonable to have some issues. cc @rxin @yhuai should we fix this?

[gatorsmile]

@lianhuiwang Writing a Hive Table in Parquet format is a little bit different here. For performance reasons, we are converting it to data source tables when inserting rows into Parquet. To get the expected results, you just need to set spark.sql.hive.convertMetastoreParquet to false.

If you are choosing textfile, it works as expected.