[
https://issues.apache.org/jira/browse/HIVE-22570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16992693#comment-16992693
]
David Mollitor commented on HIVE-22570:
---------------------------------------
So, pretty easy...
There are only 7 calls to {{getCols}} and most all of them are passed to
{{Utilities.mergeUniqElems}} which will handle the empty list just fine. The
other places check for 'null or empty' (I won't bother improving those now).
There are many more references to {{getChildren()}} however, the first few that
I sampled don't even both to check for a 'null' return value, so they are at
risk of a NPE, which is very common and why returning 'null' is generally a bad
idea. For example:
{code}
/**
* Find the variable in the expression.
* @param expr the expression to look in
* @return the index of the variable or -1 if there is not exactly one
* variable.
*/
private int findVariable(ExprNodeDesc expr) {
int result = -1;
List<ExprNodeDesc> children = expr.getChildren();
for(int i = 0; i < children.size(); ++i) {
ExprNodeDesc child = children.get(i);
if (child instanceof ExprNodeColumnDesc) {
// if we already found a variable, this isn't a sarg
if (result != -1) {
return -1;
} else {
result = i;
}
}
}
return result;
}
{code}
I can do some follow-on work to tighten up the code (and to remove all of the
'null' checks that do exist) in another ticket.
> Review of ExprNodeDesc.java
> ---------------------------
>
> Key: HIVE-22570
> URL: https://issues.apache.org/jira/browse/HIVE-22570
> Project: Hive
> Issue Type: Improvement
> Reporter: David Mollitor
> Assignee: David Mollitor
> Priority: Minor
> Attachments: HIVE-22570.1.patch
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
