[
https://issues.apache.org/jira/browse/HIVE-2530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13246499#comment-13246499
]
Phabricator commented on HIVE-2530:
-----------------------------------
kevinwilfong has commented on the revision "HIVE-2530 [jira] Implement SHOW
TBLPROPERTIES".
Looks good, just a few comments.
INLINE COMMENTS
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:370 The Table class
already has a getParameters method which does exactly this. While I agree that
the inconsistencies between the uses of the words Parameters and Properties
throughout the code is confusing, could you either call that method instead or
at least have this method return the result of getParameters(), which would at
least help to keep them consistent going forward.
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowTblPropertiesDesc.java:55
spelling
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:2573 Given that when
this feature was requested, very specific guidelines were provided for how to
format the data (one per row, tabs between property and value) it looks like
this is wanted to make it easier to parse the properties from a query. Adding
this line will make that a little more difficult.
In addition, there is a precedent for not adding a line like this in commands
like SHOW PARTITIONS and DESCRIBE which list the partitions and columns
respectively without a descriptive line like this.
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:2526-2527 I'm
guessing you got this from the implementation of DESCRIBE TABLE?
The reasoning for having that line there was so that you could describe "the
types within the column in the case when the column is an object"
https://issues.apache.org/jira/browse/HIVE-62
That shouldn't apply to this method, so you should be able to assume that the
table name is the table name, without this parsing.
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java:2533-2542 Could you
put this code in the try catch block below. The stringifyException method
prints a stack trace so developers will be able to figure out if the table was
null or not. The catch code looks identical, so this will help remove some
duplicate code.
REVISION DETAIL
https://reviews.facebook.net/D2589
> Implement SHOW TBLPROPERTIES
> ----------------------------
>
> Key: HIVE-2530
> URL: https://issues.apache.org/jira/browse/HIVE-2530
> Project: Hive
> Issue Type: New Feature
> Reporter: Adam Kramer
> Assignee: Lei Zhao
> Priority: Minor
> Attachments: HIVE-2530.D2589.1.patch
>
>
> Since table properties can be defined arbitrarily, they should be easy for a
> user to query from the command-line.
> SHOW TBLPROPERTIES tblname;
> ...would show all of them, one per row, key \t value
> SHOW TBLPROPERTIES tblname ("FOOBAR");
> ...would just show the value for the FOOBAR tblproperty.
--
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
