[ 
https://issues.apache.org/jira/browse/HIVE-7970?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Chao updated HIVE-7970:
-----------------------
    Description: 
Currently, {{Operator::removeChild}} does the following:

{code}
  public void removeChild(Operator<? extends OperatorDesc> child) {
    int childIndex = childOperators.indexOf(child);
    assert childIndex != -1;
    if (childOperators.size() == 1) {
      setChildOperators(null);
    } else {
      childOperators.remove(childIndex);
    }
    ....
{code}

However, most of the time assertion isn't turned on, and if the operator
only have one child, it may remove the wrong child.

There are lots of other method in this class that uses assertion, such as 
{{replaceChild}},
{{removeParent}}, {{replaceParent}}. They have similar problems.
I propose we change assertions to something else, maybe {{Preconditions}} in 
Guava.

  was:
Currently, {{Operator::removeChild}} does the following:

{code}
  public void removeChild(Operator<? extends OperatorDesc> child) {
    int childIndex = childOperators.indexOf(child);
    assert childIndex != -1;
    if (childOperators.size() == 1) {
      setChildOperators(null);
    } else {
      childOperators.remove(childIndex);
    }
    ....
{code}

However, most of the time assertion isn't turned on, and if the operator
only have one child, it may remove the wrong child.
I think we need to change the assertion to something else.


> Operator::removeChild may remove the wrong child
> ------------------------------------------------
>
>                 Key: HIVE-7970
>                 URL: https://issues.apache.org/jira/browse/HIVE-7970
>             Project: Hive
>          Issue Type: Bug
>            Reporter: Chao
>
> Currently, {{Operator::removeChild}} does the following:
> {code}
>   public void removeChild(Operator<? extends OperatorDesc> child) {
>     int childIndex = childOperators.indexOf(child);
>     assert childIndex != -1;
>     if (childOperators.size() == 1) {
>       setChildOperators(null);
>     } else {
>       childOperators.remove(childIndex);
>     }
>     ....
> {code}
> However, most of the time assertion isn't turned on, and if the operator
> only have one child, it may remove the wrong child.
> There are lots of other method in this class that uses assertion, such as 
> {{replaceChild}},
> {{removeParent}}, {{replaceParent}}. They have similar problems.
> I propose we change assertions to something else, maybe {{Preconditions}} in 
> Guava.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to