[ 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