Shant Hovsepian has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16428 )

Change subject: IMPALA-8291: Show constraints in DESCRIBE FORMATTED
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16428/1/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveMetadataFormatUtils.java
File 
fe/src/compat-hive-3/java/org/apache/impala/compat/HiveMetadataFormatUtils.java:

http://gerrit.cloudera.org:8080/#/c/16428/1/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveMetadataFormatUtils.java@118
PS1, Line 118:     if (!isFormatted) {
             :       for (int i = 0; i < fields.length; i++) {
             :         Object value = StringEscapeUtils.escapeJava(fields[i]);
             :         if (value != null) {
             :           tableInfo.append(value);
             :         }
             :         tableInfo.append((i == fields.length - 1) ? LINE_DELIM : 
FIELD_DELIM);
             :       }
> Looks like these are dead codes since isFormatted is always true. Are there
Yes it's just copied from Hive where it looks like dead code as well 
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java#L582

I'll remove it and add a comment.


http://gerrit.cloudera.org:8080/#/c/16428/1/fe/src/compat-hive-3/java/org/apache/impala/compat/HiveMetadataFormatUtils.java@423
PS1, Line 423: .intern()
> nit: unneccessary intern
Yup, just copied from Hive like that 
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java#L187

What do you think is it better to keep it matching Hive or update it?



--
To view, visit http://gerrit.cloudera.org:8080/16428
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I676b69c465c46491f870d7fdc894e7474c030356
Gerrit-Change-Number: 16428
Gerrit-PatchSet: 1
Gerrit-Owner: Shant Hovsepian <sh...@cloudera.com>
Gerrit-Reviewer: David Rorke <dro...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Shant Hovsepian <sh...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Comment-Date: Sat, 12 Sep 2020 18:19:31 +0000
Gerrit-HasComments: Yes

Reply via email to