[
https://issues.apache.org/jira/browse/HIVE-19970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16521983#comment-16521983
]
Peter Vary commented on HIVE-19970:
-----------------------------------
[~maheshk114]: Thanks for fixing the test
Not an expert on replication but a few comments anyway on coding style which
might be worth to fix:
* If you have to change the patch, it would be good to take care of the
checkstyle errors shown by yetus results:
{code}
./itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosAcrossInstances.java:1103:
CommandProcessorResponse ret = replica.runCommand("REPL LOAD " +
replicatedDbName + " FROM '" + tuple2.dumpLocation + "'");: warning: Line is
longer than 120 characters (found 127).
./itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenarios.java:3195:
Path path = new Path(System.getProperty("test.warehouse.dir",""));:66:
warning: ',' is not followed by whitespace.
./standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java:4991:
throw new InvalidOperationException("Unable to alter table" + dbname
+ "." + name + " as new db name is null");: warning: Line is longer than 120
characters (found 122).
{code}
* Changes in {{HiveAlterHandler.java}} - these are not necessary, and I usually
try to avoid them if I do not change the code anyway. When someone cherry-picks
a change these changes can make ones life a misery.
* I see several cases when we log something in error, but immediately rethrow
the exception. If the rethrown exceptions are logged later then this extra log
line is unnecessary, and makes reading the log harder. Since I am not an expert
of the code here I do not know if this is the case or not.
* Also I usually try to avoid cramming two changes in one fix (Repl fix, and
unrelated test fix) - The reason behind is again that if we have to cherry-pick
the change we might want only one of the changes and not both
As mentioned above, I am not an expert on Replication, so if the other
reviewers suggest changes, the it would be nice to fix these as well.
Thanks,
Peter
PS: Rechecked the {{TestTablesCreateDropAlterTruncate.
testAlterTableNullDatabaseInNew}} one step deeper, and the "funny" thing is
that your change in HIVE-19569 actually reverted the change caused by
HIVE-19418. I will ask the developers of that patch about the reasoning.
> Replication dump has a NPE when table is empty
> ----------------------------------------------
>
> Key: HIVE-19970
> URL: https://issues.apache.org/jira/browse/HIVE-19970
> Project: Hive
> Issue Type: Task
> Components: repl
> Affects Versions: 3.1.0, 4.0.0
> Reporter: mahesh kumar behera
> Assignee: mahesh kumar behera
> Priority: Major
> Labels: pull-request-available
> Fix For: 3.1.0, 4.0.0
>
> Attachments: HIVE-19970.01.patch
>
>
> if table directory or partition directory is missing ..dump is throwing NPE
> instead of file missing exception.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
