[SPARK-16959] [SQL] Rebuild Table Comment when Retrieving Metadata from Hive Metastore #14550

gatorsmile · 2016-08-08T23:45:28Z

What changes were proposed in this pull request?

The comment in CatalogTable returned from Hive is always empty. We store it in the table property when creating a table. However, when we try to retrieve the table metadata from Hive metastore, we do not rebuild it. The comment is always empty.

This PR is to fix the issue.

How was this patch tested?

Fixed the test case to verify the change.

SparkQA · 2016-08-09T01:24:03Z

Test build #63392 has finished for PR 14550 at commit e546af5.

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

gatorsmile · 2016-08-09T02:32:05Z

cc @cloud-fan . This is a tiny fix. Could you review this?

cloud-fan · 2016-08-09T03:42:55Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

        val tableMetadata = catalog.getTableMetadata(TableIdentifier(tabName, Some("default")))
        val viewMetadata = catalog.getTableMetadata(TableIdentifier(viewName, Some("default")))
+        assert(tableMetadata.comment == Option("BLABLA"))
        assert(tableMetadata.properties.get("comment") == Option("BLABLA"))


I think we should also remove the comment from table properties, to not surprise users.

As explained below, Hive keeps comment in the table properties. Should we keep it too?

Thanks!

I don't think so. HiveClient should be symmetrical, the table meta read back should be same with what users saved into. We don't need to follow hive here.

Ok. I see. Let me fix it now.

cloud-fan · 2016-08-09T03:43:39Z

do you know why we put comment in table properties instead of table comment field? Some hive tricks?

gatorsmile · 2016-08-09T20:26:28Z

After an investigation, Hive does not have such a field in org.apache.hadoop.hive.ql.metadata.Table. Thus, they also put it in the table properties/parameters.

For example, this is the Hive output of one table with table comment d

hive> desc formatted tab5;
OK
# col_name              data_type               comment             

id                      int                     b                   

# Detailed Table Information         
Database:               default                  
Owner:                  root                     
CreateTime:             Mon Aug 08 16:19:12 EDT 2016     
LastAccessTime:         UNKNOWN                  
Protect Mode:           None                     
Retention:              0                        
Location:               hdfs://6b68a24121f4:9000/user/hive/warehouse/tab5    
Table Type:             MANAGED_TABLE            
Table Parameters:        
    comment                 d                   
    transient_lastDdlTime   1470687552          

# Storage Information        
SerDe Library:          org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe   
InputFormat:            org.apache.hadoop.mapred.TextInputFormat     
OutputFormat:           org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat   
Compressed:             No                       
Num Buckets:            -1                       
Bucket Columns:         []                       
Sort Columns:           []                       
Storage Desc Params:         
    serialization.format    1

cloud-fan · 2016-08-10T06:51:40Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

            .map(_.asScala.toMap).orNull
        ),
-        properties = properties,
+        properties = properties.filter(kv => kv._1 != "path"),


why not use filterKeys?

The implementation of filterKeys will create a new object.

override def filterKeys(p: A => Boolean): Map[A, B] = new FilteredKeys(p) with DefaultMap[A, B]

Thus, I got the following error:

org.apache.spark.SparkException: Task not serializable

hey, we should filter out comment here, not path

haha, sure. Working on multiple PRs at the same time. Will open a new PR tomorrow regarding path...

SparkQA · 2016-08-10T08:03:29Z

Test build #63508 has finished for PR 14550 at commit e575700.

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

cloud-fan · 2016-08-10T08:27:53Z

thanks, merging to master!

SparkQA · 2016-08-10T09:38:52Z

Test build #63517 has finished for PR 14550 at commit b1cbf55.

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

gatorsmile · 2016-09-03T03:51:43Z

@cloud-fan To backport #14531, we need to backport this. Do you want me to do it?

…m Hive Metastore ### What changes were proposed in this pull request? The `comment` in `CatalogTable` returned from Hive is always empty. We store it in the table property when creating a table. However, when we try to retrieve the table metadata from Hive metastore, we do not rebuild it. The `comment` is always empty. This PR is to fix the issue. ### How was this patch tested? Fixed the test case to verify the change. Author: gatorsmile <gatorsmile@gmail.com> Closes #14550 from gatorsmile/tableComment. (cherry picked from commit bdd5371) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

cloud-fan · 2016-09-03T16:11:17Z

backported to 2.0

gatorsmile · 2016-09-03T17:39:42Z

Thank you! Let me fix the test case failure in Branch 2.0

fix.

e546af5

cloud-fan reviewed Aug 9, 2016
View reviewed changes

cloud-fan mentioned this pull request Aug 10, 2016

[SPARK-17353] [SPARK-16943] [SPARK-16942] [SQL] Fix multiple bugs in CREATE TABLE LIKE command #14531

Closed

gatorsmile added 3 commits August 9, 2016 23:03

fix.

6ed3374

Merge remote-tracking branch 'upstream/master' into tableComment

0350561

replace filterKey by filter

e575700

cloud-fan reviewed Aug 10, 2016
View reviewed changes

address comments

b1cbf55

asfgit closed this in bdd5371 Aug 10, 2016

gatorsmile mentioned this pull request Sep 3, 2016

[SPARK-17353] [SPARK-16943] [SPARK-16942] [BACKPORT-2.0] [SQL] Fix multiple bugs in CREATE TABLE LIKE command #14946

Closed

[SPARK-16959] [SQL] Rebuild Table Comment when Retrieving Metadata from Hive Metastore #14550

[SPARK-16959] [SQL] Rebuild Table Comment when Retrieving Metadata from Hive Metastore #14550

Uh oh!

Conversation

gatorsmile commented Aug 8, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Aug 9, 2016

Uh oh!

gatorsmile commented Aug 9, 2016

Uh oh!

cloud-fan Aug 9, 2016

Choose a reason for hiding this comment

Uh oh!

gatorsmile Aug 9, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan Aug 10, 2016

Choose a reason for hiding this comment

Uh oh!

gatorsmile Aug 10, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Aug 9, 2016

Uh oh!

gatorsmile commented Aug 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan Aug 10, 2016

Choose a reason for hiding this comment

Uh oh!

gatorsmile Aug 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan Aug 10, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan Aug 10, 2016

Choose a reason for hiding this comment

Uh oh!

gatorsmile Aug 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 10, 2016

Uh oh!

cloud-fan commented Aug 10, 2016

Uh oh!

SparkQA commented Aug 10, 2016

Uh oh!

gatorsmile commented Sep 3, 2016

Uh oh!

cloud-fan commented Sep 3, 2016

Uh oh!

gatorsmile commented Sep 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gatorsmile commented Aug 9, 2016 •

edited

Loading

gatorsmile Aug 10, 2016 •

edited

Loading

gatorsmile Aug 10, 2016 •

edited

Loading

gatorsmile commented Sep 3, 2016 •

edited

Loading