[ 
https://issues.apache.org/jira/browse/HIVE-2822?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13238967#comment-13238967
 ] 

Phabricator commented on HIVE-2822:
-----------------------------------

ashutoshc has requested changes to the revision "HIVE-2822 [jira] Add JSON 
output to the hive ddl commands".

  Since All the new introduced files deals with formatting output. I think they 
should be move to org.apache.hadoop.hive.ql.metadata.formatting package. 
Perhaps MetaDataFormatUtils.java should also be moved into this new package.

  Also, it would be good to add negative test cases in clientnegative for 
failure scenarios, like adding/dropping a table which doesn't exist.

INLINE COMMENTS
  ql/src/java/org/apache/hadoop/hive/ql/metadata/MetaDataFormatter.java:23 
Unused import.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/MetaDataFormatter.java:40 
Since there are only handful of them, will enum be a better choice here than 
ints?
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:397 Are you sure you 
want stack-trace here? In case, table already exists you just want to say that 
to user, not the full stack trace.
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:402 Are you sure you 
want stack-trace here? In case, db/table/partition doesnt exist you just want 
to say that to user, not the full stack trace.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/MetaDataFormatter.java:38 
Since all of the methods in the interface takes an output stream as an arg and 
writes to it, so interface is both formatter as well as writer. So, I wonder if 
better name for the interface is FormattedMetaDataWriter or something like that?
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:1855 For 
showDatabases/showTables you have made changes even for these exceptions. To 
keep it symmetrical, it makes sense to have these messages also go through 
formatter.
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:1858 For 
showDatabases/showTables you have made changes even for these exceptions. To 
keep it symmetrical, it makes sense to have these messages also go through 
formatter.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/TextMetaDataFormatter.java:127 
Whats the advantage of splitting it into two methods. You might as well inline 
content of this method here.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/TextMetaDataFormatter.java:199 
Whats the advantage of splitting it into two methods. You might as well inline 
content of this method here.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/MapBuilder.java:28 This map 
can be parameterized to <String,Object>
  ql/src/java/org/apache/hadoop/hive/ql/metadata/JsonMetaDataFormatter.java:163 
Return type should be List<FieldSchema>
  ql/src/java/org/apache/hadoop/hive/ql/metadata/JsonMetaDataFormatter.java:193 
Genricize the return type.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/JsonMetaDataFormatter.java:210 
Genricize the return type.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/JsonMetaDataFormatter.java:375 
Genricize the return type.
  ql/src/java/org/apache/hadoop/hive/ql/metadata/JsonMetaDataFormatter.java:389 
Genricize the return type.
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:191 This config key 
should be defined into HiveConf.java and should be added in 
hive-site.xml.template. Also, instead of "hive.format" better name of this key 
would be "hive.ddl.output.format"

REVISION DETAIL
  https://reviews.facebook.net/D2475

BRANCH
  HIVE-2822-dev-branch

                
> Add JSON output to the hive ddl commands
> ----------------------------------------
>
>                 Key: HIVE-2822
>                 URL: https://issues.apache.org/jira/browse/HIVE-2822
>             Project: Hive
>          Issue Type: Improvement
>            Reporter: Chris Dean
>            Assignee: Chris Dean
>         Attachments: HIVE-2822.03-branch0-8.patch, HIVE-2822.03.patch, 
> HIVE-2822.03b.patch, HIVE-2822.04-branch-08.patch, 
> HIVE-2822.05-branch0-8-1.patch, HIVE-2822.05-branch0-8.patch, 
> HIVE-2822.05.patch, HIVE-2822.D2475.1.patch, hive-json-01-branch0-8.patch, 
> hive-json-01.patch, hive-json-02-branch0-8.patch, hive-json-02.patch
>
>
> The goal is to have an option to produce JSON output of the DDL commands that 
> is easily machine parseable.
> For example, "desc my_table" currently gives
> {noformat}
>     id    bigint
>     user  string
> {noformat} 
> and we want to allow a json output:
> {noformat}
>     {
>       "columns": [
>         {"name": "id", "type": "bigint"},
>         {"name": "user", "type": "string"}
>       ]
>     }
> {noformat} 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to