HIVE-29474: DESCRIBE FORMATTED for columns produces wrong output when…

itests/hive-unit/src/test/java/org/apache/hadoop/hive/serde2/TestSerdeWithFieldComments.java

@@ -68,7 +68,7 @@ public void testFieldComments() throws MetaException, SerDeException {
     Deserializer mockDe = mock(Deserializer.class);
     when(mockDe.getObjectInspector()).thenReturn(mockSOI);
     List<FieldSchema> result =
-        HiveMetaStoreUtils.getFieldsFromDeserializer("testTable", mockDe, new HiveConf());
+        HiveMetaStoreUtils.getFieldsFromDeserializer("testTable", mockDe, new HiveConf(), false);
 
     assertEquals(2, result.size());
     assertEquals("first", result.get(0).getName());

metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreUtils.java

@@ -155,7 +155,7 @@ static public Deserializer getDeserializer(Configuration conf,
    * @throws MetaException if one of the fields or types in the table is invalid
    */
   public static List<FieldSchema> getFieldsFromDeserializer(String tableName,
-      Deserializer deserializer, Configuration conf) throws SerDeException, MetaException {
+      Deserializer deserializer, Configuration conf, boolean isDescFormColsCmd) throws SerDeException, MetaException {
     Collection<String> noCommentSerdes =
         MetastoreConf.getStringCollection(conf, MetastoreConf.ConfVars.SERDES_WITHOUT_FROM_DESERIALIZER);
     boolean noCommentFromDeserializer = noCommentSerdes.contains(deserializer.getClass().getName());
@@ -191,7 +191,7 @@ public static List<FieldSchema> getFieldsFromDeserializer(String tableName,
 
     ArrayList<FieldSchema> str_fields = new ArrayList<>();
     // rules on how to recurse the ObjectInspector based on its type
-    if (oi.getCategory() != Category.STRUCT) {
+    if (isDescFormColsCmd || oi.getCategory() != Category.STRUCT) {
       str_fields.add(new FieldSchema(last_name, oi.getTypeName(),
           determineFieldComment(null, noCommentFromDeserializer)));
     } else {

metastore/src/java/org/apache/hadoop/hive/metastore/SerDeStorageSchemaReader.java

@@ -44,7 +44,7 @@ public List<FieldSchema> readSchema(Table tbl, EnvironmentContext envContext, Co
       }
 
       Deserializer s = HiveMetaStoreUtils.getDeserializer(conf, tbl, null, false);
-      return HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), s, conf);
+      return HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), s, conf, false);
     } catch (Exception e) {
       throw new MetaException(e.getMessage());
     } finally {

ql/src/java/org/apache/hadoop/hive/ql/Compiler.java

@@ -414,7 +414,7 @@ private void setSchema(BaseSemanticAnalyzer sem) {
         List<FieldSchema> lst = null;
         try {
           lst = HiveMetaStoreUtils.getFieldsFromDeserializer(tableName, td.getDeserializer(driverContext.getConf()),
-              driverContext.getConf());
+              driverContext.getConf(), false);
         } catch (Exception e) {
           LOG.warn("Error getting schema", e);
         }

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/update/AlterTableUpdateColumnsOperation.java

@@ -58,7 +58,7 @@ protected void doAlteration(Table table, Partition partition) throws HiveExcepti
     try {
       LOG.info("Updating metastore columns for table: {}", table.getTableName());
       List<FieldSchema> fields = HiveMetaStoreUtils.getFieldsFromDeserializer(table.getTableName(), deserializer,
-          context.getConf());
+          context.getConf(), false);
       StorageDescriptor sd = getStorageDescriptor(table, partition);
       sd.setCols(fields);
     } catch (org.apache.hadoop.hive.serde2.SerDeException | MetaException e) {

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/desc/DescTableOperation.java

@@ -91,7 +91,7 @@ public int execute() throws Exception {
         if (desc.isFormatted()) {
           getColumnDataColPathSpecified(table, part, cols, colStats, deserializer);
         } else {
-          cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf()));
+          cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf(), false));
         }
       }
       fixDecimalColumnTypeName(cols);
@@ -204,14 +204,14 @@ private void getColumnDataColPathSpecified(Table table, Partition part, List<Fie
         }
         table.setParameters(tableProps);
       } else {
-        cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf()));
+        cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf(), true));
         colStats.addAll(context.getDb().getTableColumnStatistics(table, colNames, false));
       }
     } else {
       List<String> partitions = new ArrayList<>();
       String partName = part.getName();
       partitions.add(partName);
-      cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf()));
+      cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf(), true));
       Map<String, List<ColumnStatisticsObj>> partitionColumnStatistics = context.getDb().getPartitionColumnStatistics(
           table.getDbName(), table.getTableName(), partitions, colNames, false);
       List<ColumnStatisticsObj> partitionColStat = partitionColumnStatistics.get(partName);
@@ -241,7 +241,7 @@ private void getColumnDataForPartitionKeyColumn(Table table, List<FieldSchema> c
   private void getColumnsForNotPartitionKeyColumn(Table table, List<FieldSchema> cols, List<ColumnStatisticsObj> colStats,
       Deserializer deserializer, List<String> colNames, Map<String, String> tableProps)
       throws HiveException {
-    cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf()));
+    cols.addAll(Hive.getFieldsFromDeserializer(desc.getColumnPath(), deserializer, context.getConf(), true));
     List<String> parts = context.getDb().getPartitionNames(table, (short) -1);
 
     AggrStats aggrStats = context.getDb().getAggrColStatsFor(table, colNames, parts, false);

ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/serde/AlterTableSetSerdeOperation.java

@@ -69,7 +69,7 @@ protected void doAlteration(Table table, Partition partition) throws HiveExcepti
         try {
           Deserializer oldSerde = HiveMetaStoreUtils.getDeserializer(context.getConf(), table.getTTable(), null,
               false, oldSerdeName);
-          table.setFields(Hive.getFieldsFromDeserializer(table.getTableName(), oldSerde, context.getConf()));
+          table.setFields(Hive.getFieldsFromDeserializer(table.getTableName(), oldSerde, context.getConf(), false));
         } catch (MetaException ex) {
           throw new HiveException(ex);
         }

ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java

@@ -1371,7 +1371,8 @@ public void createTable(Table tbl, boolean ifNotExists,
         tbl.setDbName(SessionState.get().getCurrentDatabase());
       }
       if (tbl.getCols().size() == 0 || tbl.getSd().getColsSize() == 0) {
-        tbl.setFields(HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), tbl.getDeserializer(), conf));
+        tbl.setFields(HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), tbl.getDeserializer(),
+            conf, false));
       }
       tbl.checkValidity(conf);
       if (tbl.getParameters() != null) {
@@ -1450,7 +1451,8 @@ public void createTable(Table tbl, boolean ifNotExists) throws HiveException {
 
   public static List<FieldSchema> getFieldsFromDeserializerForMsStorage(
       Table tbl, Deserializer deserializer, Configuration conf) throws SerDeException, MetaException {
-    List<FieldSchema> schema = HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), deserializer, conf);
+    List<FieldSchema> schema = HiveMetaStoreUtils.getFieldsFromDeserializer(tbl.getTableName(), deserializer,
+        conf, false);
     for (FieldSchema field : schema) {
       field.setType(MetaStoreUtils.TYPE_FROM_DESERIALIZER);
     }
@@ -6211,10 +6213,11 @@ private List<String> getGroupNames() {
     return null;
   }
 
-  public static List<FieldSchema> getFieldsFromDeserializer(String name, Deserializer serde, Configuration conf)
+  public static List<FieldSchema> getFieldsFromDeserializer(String name, Deserializer serde, Configuration conf, 
+      boolean isDescFormColsCmd)
       throws HiveException {
     try {
-      return HiveMetaStoreUtils.getFieldsFromDeserializer(name, serde, conf);
+      return HiveMetaStoreUtils.getFieldsFromDeserializer(name, serde, conf, isDescFormColsCmd);
     } catch (SerDeException e) {
       throw new HiveException("Error in getting fields from serde. " + e.getMessage(), e);
     } catch (MetaException e) {

ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java

@@ -511,7 +511,7 @@ private List<FieldSchema> getColsInternal(boolean forMs) {
         return Hive.getFieldsFromDeserializerForMsStorage(table, getDeserializer(), SessionState.getSessionConf());
       }
       return HiveMetaStoreUtils.getFieldsFromDeserializer(table.getTableName(), getDeserializer(),
-          SessionState.getSessionConf());
+          SessionState.getSessionConf(), false);
     } catch (Exception e) {
       LOG.error("Unable to get cols from serde: " +
           tPartition.getSd().getSerdeInfo().getSerializationLib(), e);

ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java

@@ -761,7 +761,7 @@ private List<FieldSchema> getColsInternal(boolean forMs) {
         return Hive.getFieldsFromDeserializerForMsStorage(this, getDeserializer(), SessionState.getSessionConf());
       } else {
         return HiveMetaStoreUtils.getFieldsFromDeserializer(getTableName(), getDeserializer(),
-            SessionState.getSessionConf());
+            SessionState.getSessionConf(), false);
       }
     } catch (Exception e) {
       LOG.error("Unable to get field from serde: " + serializationLib, e);

ql/src/test/queries/clientpositive/desc_cols_formatted.q

@@ -0,0 +1,15 @@
+CREATE TABLE tbl_t (id int, point STRUCT<x:INT, y:INT>);
+
+DESCRIBE FORMATTED tbl_t;
+
+DESCRIBE FORMATTED tbl_t point;
+
+DESCRIBE tbl_t point;
+
+CREATE TABLE tbl_part(id int, point STRUCT<x:INT, y:INT>) PARTITIONED BY (name string);
+
+DESCRIBE FORMATTED tbl_part;
+
+DESCRIBE FORMATTED tbl_part point;
+
+DESCRIBE tbl_part point;

ql/src/test/results/clientpositive/llap/desc_cols_formatted.q.out

@@ -0,0 +1,146 @@
+PREHOOK: query: CREATE TABLE tbl_t (id int, point STRUCT<x:INT, y:INT>)
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl_t
+POSTHOOK: query: CREATE TABLE tbl_t (id int, point STRUCT<x:INT, y:INT>)
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl_t
+PREHOOK: query: DESCRIBE FORMATTED tbl_t
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_t
+POSTHOOK: query: DESCRIBE FORMATTED tbl_t
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_t
+# col_name            	data_type           	comment             
+id                  	int                 	                    
+point               	struct<x:int,y:int> 	                    
+
+# Detailed Table Information	 	 
+Database:           	default             	 
+#### A masked pattern was here ####
+Retention:          	0                   	 
+#### A masked pattern was here ####
+Table Type:         	MANAGED_TABLE       	 
+Table Parameters:	 	 
+	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"point\":\"true\"}}
+	bucketing_version   	2                   
+	numFiles            	0                   
+	numRows             	0                   
+	rawDataSize         	0                   
+	totalSize           	#Masked#
+#### A masked pattern was here ####
+
+# 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                   
+PREHOOK: query: DESCRIBE FORMATTED tbl_t point
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_t
+POSTHOOK: query: DESCRIBE FORMATTED tbl_t point
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_t
+col_name            	point               
+data_type           	struct<x:int,y:int> 
+min                 	                    
+max                 	                    
+num_nulls           	                    
+distinct_count      	                    
+avg_col_len         	                    
+max_col_len         	                    
+num_trues           	                    
+num_falses          	                    
+bit_vector          	                    
+comment             	from deserializer   
+COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"point\":\"true\"}}
+PREHOOK: query: DESCRIBE tbl_t point
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_t
+POSTHOOK: query: DESCRIBE tbl_t point
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_t
+x                   	int                 	from deserializer   
+y                   	int                 	from deserializer   
+COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"point\":\"true\"}}	 
+PREHOOK: query: CREATE TABLE tbl_part(id int, point STRUCT<x:INT, y:INT>) PARTITIONED BY (name string)
+PREHOOK: type: CREATETABLE
+PREHOOK: Output: database:default
+PREHOOK: Output: default@tbl_part
+POSTHOOK: query: CREATE TABLE tbl_part(id int, point STRUCT<x:INT, y:INT>) PARTITIONED BY (name string)
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: database:default
+POSTHOOK: Output: default@tbl_part
+PREHOOK: query: DESCRIBE FORMATTED tbl_part
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_part
+POSTHOOK: query: DESCRIBE FORMATTED tbl_part
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_part
+# col_name            	data_type           	comment             
+id                  	int                 	                    
+point               	struct<x:int,y:int> 	                    
+
+# Partition Information	 	 
+# col_name            	data_type           	comment             
+name                	string              	                    
+
+# Detailed Table Information	 	 
+Database:           	default             	 
+#### A masked pattern was here ####
+Retention:          	0                   	 
+#### A masked pattern was here ####
+Table Type:         	MANAGED_TABLE       	 
+Table Parameters:	 	 
+	COLUMN_STATS_ACCURATE	{\"BASIC_STATS\":\"true\"}
+	bucketing_version   	2                   
+	numFiles            	0                   
+	numPartitions       	0                   
+	numRows             	0                   
+	rawDataSize         	0                   
+	totalSize           	#Masked#
+#### A masked pattern was here ####
+
+# 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                   
+PREHOOK: query: DESCRIBE FORMATTED tbl_part point
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_part
+POSTHOOK: query: DESCRIBE FORMATTED tbl_part point
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_part
+col_name            	point               
+data_type           	struct<x:int,y:int> 
+min                 	                    
+max                 	                    
+num_nulls           	                    
+distinct_count      	                    
+avg_col_len         	                    
+max_col_len         	                    
+num_trues           	                    
+num_falses          	                    
+bit_vector          	                    
+comment             	from deserializer   
+COLUMN_STATS_ACCURATE	{\"COLUMN_STATS\":{\"point\":\"true\"}}
+PREHOOK: query: DESCRIBE tbl_part point
+PREHOOK: type: DESCTABLE
+PREHOOK: Input: default@tbl_part
+POSTHOOK: query: DESCRIBE tbl_part point
+POSTHOOK: type: DESCTABLE
+POSTHOOK: Input: default@tbl_part
+x                   	int                 	from deserializer   
+y                   	int                 	from deserializer