[ 
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, it's often the case that assertion isn't enabled, and if the operator
only have one child, it may remove the wrong child.

There are lots of other methods in this class that use assertion, such as 
{{replaceChild}},
{{removeParent}}, {{replaceParent}}, etc. They have similar problems.
I propose we change these 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.

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.


> 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, it's often the case that assertion isn't enabled, and if the operator
> only have one child, it may remove the wrong child.
> There are lots of other methods in this class that use assertion, such as 
> {{replaceChild}},
> {{removeParent}}, {{replaceParent}}, etc. They have similar problems.
> I propose we change these assertions to something else, maybe 
> {{Preconditions}} in Guava.



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

Reply via email to