[
https://issues.apache.org/jira/browse/HIVE-26456?focusedWorklogId=802489&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-802489
]
ASF GitHub Bot logged work on HIVE-26456:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Aug/22 14:32
Start Date: 22/Aug/22 14:32
Worklog Time Spent: 10m
Work Description: zabetak commented on code in PR #3506:
URL: https://github.com/apache/hive/pull/3506#discussion_r951498152
##########
accumulo-handler/src/test/org/apache/hadoop/hive/accumulo/predicate/TestAccumuloPredicateHandler.java:
##########
@@ -386,7 +386,7 @@ public void testPushdownComparisonOptNotSupported() {
} catch (RuntimeException e) {
assertTrue(e.getMessage().contains("Unexpected residual predicate:
field1 is not null"));
} catch (Exception e) {
- fail(StringUtils.stringifyException(e));
+ fail(e.getMessage());
}
Review Comment:
Remove unused `import org.apache.hadoop.util.StringUtils;`
##########
accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/AccumuloStorageHandler.java:
##########
@@ -369,11 +369,7 @@ public void preCreateTable(Table table) throws
MetaException {
tableOpts.create(idxTable);
}
}
- } catch (AccumuloSecurityException e) {
- throw new MetaException(StringUtils.stringifyException(e));
- } catch (TableExistsException e) {
- throw new MetaException(StringUtils.stringifyException(e));
- } catch (AccumuloException e) {
+ } catch (AccumuloSecurityException | TableExistsException |
AccumuloException e) {
Review Comment:
In a previous PR (https://github.com/apache/hive/pull/3478) we opted to use
the LOG + throw pattern as an alternative of removing `stringifyException`
acknowledging advantages and disadvantages of it. Why we don't want to do the
same here?
##########
accumulo-handler/src/test/org/apache/hadoop/hive/accumulo/predicate/TestAccumuloPredicateHandler.java:
##########
@@ -386,7 +386,7 @@ public void testPushdownComparisonOptNotSupported() {
} catch (RuntimeException e) {
assertTrue(e.getMessage().contains("Unexpected residual predicate:
field1 is not null"));
} catch (Exception e) {
- fail(StringUtils.stringifyException(e));
+ fail(e.getMessage());
}
Review Comment:
It seems we don't really need this catch.
##########
accumulo-handler/src/test/org/apache/hadoop/hive/accumulo/predicate/TestAccumuloPredicateHandler.java:
##########
@@ -475,7 +474,7 @@ public void testIgnoreIteratorPushdown() throws
TooManyAccumuloColumnsException
List<IteratorSetting> iterators = handler.getIterators(conf,
columnMapper);
assertEquals(iterators.size(), 0);
} catch (Exception e) {
- fail(StringUtils.stringifyException(e));
+ fail(e.getMessage());
}
Review Comment:
Same comment as before.
##########
druid-handler/src/test/org/apache/hadoop/hive/druid/QTestDruidSerDe.java:
##########
@@ -96,7 +96,7 @@ public class QTestDruidSerDe extends DruidSerDe {
DruidStorageHandlerUtils.JSON_MAPPER.readValue(RESPONSE, new
TypeReference<List<SegmentAnalysis>>() {
});
} catch (Exception e) {
- throw new SerDeException(StringUtils.stringifyException(e));
+ throw new SerDeException(e);
Review Comment:
Remove unused `import org.apache.hadoop.util.StringUtils;`
##########
accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/mr/HiveAccumuloTableInputFormat.java:
##########
@@ -175,15 +175,8 @@ public InputSplit[] getSplits(JobConf jobConf, int
numSplits) throws IOException
}
return hiveSplits;
- } catch (AccumuloException e) {
- log.error("Could not configure AccumuloInputFormat", e);
- throw new IOException(StringUtils.stringifyException(e));
- } catch (AccumuloSecurityException e) {
- log.error("Could not configure AccumuloInputFormat", e);
- throw new IOException(StringUtils.stringifyException(e));
- } catch (SerDeException e) {
- log.error("Could not configure AccumuloInputFormat", e);
- throw new IOException(StringUtils.stringifyException(e));
+ } catch (AccumuloException | AccumuloSecurityException | SerDeException e)
{
+ throw new IOException("Could not configure AccumuloInputFormat", e);
Review Comment:
Remove unused `import org.apache.hadoop.util.StringUtils;`
##########
accumulo-handler/src/test/org/apache/hadoop/hive/accumulo/predicate/TestAccumuloPredicateHandler.java:
##########
@@ -423,7 +423,6 @@ public void testIteratorIgnoreRowIDFields() {
List<IteratorSetting> iterators = handler.getIterators(conf,
columnMapper);
assertEquals(iterators.size(), 0);
} catch (SerDeException e) {
- StringUtils.stringifyException(e);
}
Review Comment:
Ignoring the exception seems wrong. Most likely the test should fail in case
of exception. Can we fix this as part of this PR?
Issue Time Tracking
-------------------
Worklog Id: (was: 802489)
Time Spent: 50m (was: 40m)
> Remove stringifyException Method From Storage Handlers
> ------------------------------------------------------
>
> Key: HIVE-26456
> URL: https://issues.apache.org/jira/browse/HIVE-26456
> Project: Hive
> Issue Type: Sub-task
> Reporter: David Mollitor
> Assignee: David Mollitor
> Priority: Major
> Labels: pull-request-available
> Time Spent: 50m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
