[
https://issues.apache.org/jira/browse/HIVE-26469?focusedWorklogId=802483&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-802483
]
ASF GitHub Bot logged work on HIVE-26469:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Aug/22 14:04
Start Date: 22/Aug/22 14:04
Worklog Time Spent: 10m
Work Description: zabetak commented on code in PR #3518:
URL: https://github.com/apache/hive/pull/3518#discussion_r951459418
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/ReduceRecordSource.java:
##########
@@ -373,8 +370,7 @@ public void next() throws HiveException {
try {
rowString = SerDeUtils.getJSONString(row, rowObjectInspector);
} catch (Exception e2) {
- rowString = "[Error getting row data with exception "
- + StringUtils.stringifyException(e2) + " ]";
+ l4j.trace("Error getting row data (tag={})", tag, e2);
Review Comment:
The refactoring in `ExecReducer` was a bit different. Maybe it makes sense
to keep things uniform.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/ReduceRecordSource.java:
##########
@@ -502,8 +495,7 @@ private void processVectorGroup(BytesWritable keyWritable,
try {
rowString = batch.toString();
} catch (Exception e2) {
- rowString = "[Error getting row data with exception "
- + StringUtils.stringifyException(e2) + " ]";
+ l4j.error("Error getting row data (tag={})", tag, e2);
Review Comment:
The refactoring in `ExecReducer` is different.
Moreover, previously we used `l4j.trace` and here we use `l4j.error`. The
discussion in `HIVE-20644` indicates that maybe it is safer to log at trace
level when it comes to data. This is outside the scope of this PR but maybe
worth filing a JIRA case for this.
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java:
##########
@@ -286,11 +286,7 @@ public void iterate(AggregationBuffer agg, Object[]
parameters) throws HiveExcep
} catch (NumberFormatException e) {
if (!warned) {
warned = true;
- LOG.warn(getClass().getSimpleName() + " "
- + StringUtils.stringifyException(e));
- LOG
- .warn(getClass().getSimpleName()
- + " ignoring similar exceptions.");
+ LOG.warn("{}: ignoring similar exceptions",
getClass().getSimpleName(), e);
Review Comment:
Remove unused `import org.apache.hadoop.util.StringUtils;`
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java:
##########
@@ -286,11 +286,7 @@ public void iterate(AggregationBuffer agg, Object[]
parameters) throws HiveExcep
} catch (NumberFormatException e) {
if (!warned) {
warned = true;
- LOG.warn(getClass().getSimpleName() + " "
- + StringUtils.stringifyException(e));
- LOG
- .warn(getClass().getSimpleName()
- + " ignoring similar exceptions.");
+ LOG.warn("{}: ignoring similar exceptions",
getClass().getSimpleName(), e);
Review Comment:
The `getClass().getSimpleName()` is a bit useless/redundant for two reasons:
* The class name will appear in the stacktrace;
* The log configuration can be changed to include the class name if
necessary.
The comment applies to all changes in this class.
The proposed refactoring though simply retains the old behavior so this is
not a blocking comment.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/ReduceRecordSource.java:
##########
@@ -398,15 +394,12 @@ private boolean pushRecordVector() {
processVectorGroup(keyWritable, valueWritables, tag);
return true;
- } catch (Throwable e) {
+ } catch (OutOfMemoryError oom) {
abort = true;
- if (e instanceof OutOfMemoryError) {
- // Don't create a new object if we are already out of memory
- throw (OutOfMemoryError) e;
- } else {
- l4j.error(StringUtils.stringifyException(e));
- throw new RuntimeException(e);
- }
+ // Do not create a new object if we are already out of memory
+ throw oom;
+ } catch (Throwable t) {
+ throw new RuntimeException(t);
Review Comment:
`abort = true;` is missing here. This is the only major review comment in
this PR.
Issue Time Tracking
-------------------
Worklog Id: (was: 802483)
Time Spent: 20m (was: 10m)
> Remove stringifyException Method From QL Package
> ------------------------------------------------
>
> Key: HIVE-26469
> URL: https://issues.apache.org/jira/browse/HIVE-26469
> Project: Hive
> Issue Type: Sub-task
> Reporter: David Mollitor
> Assignee: David Mollitor
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
