[jira] [Updated] (NIFI-2979) PriorityAttributePrioritizer violates Comparator contract

["Brandon DeVries (JIRA)" <ji...@apache.org>]

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

Brandon DeVries updated NIFI-2979:
----------------------------------
    Description: 
The documentation for the compare() method of the Comparator interface\[1] states:

{quote}
The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y.
{quote}

However, in the PriorityAttributePrioritizer\[2], we have the following snippet:

{code}
String o1Priority = o1.getAttribute(CoreAttributes.PRIORITY.key());
String o2Priority = o2.getAttribute(CoreAttributes.PRIORITY.key());
if (o1Priority == null && o2Priority == null) {
     return -1; // this is not 0 to match FirstInFirstOut
} else if (o2Priority == null) {
     return -1;
} else if (o1Priority == null) {
     return 1;
}
{code}

This implies that for two non-null FlowFiles f1 and f2, both with null "priority" attributes, we would have:

{code}
compare(f1, f2) == -1
compare(f2, f1) == -1
{code}

This would appear to violate the contract of the Comparator interface.  The comment suggests this was done to preserve FIFO ordering, however a return of 0 should do the same, and satisfy the contract.

\[1] https://docs.oracle.com/javase/7/docs/api/java/util/Comparator.html#compare%28T,%20T%29
\[2] https://github.com/apache/nifi/blob/d838f61291d2582592754a37314911b701c6891b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-prioritizers/src/main/java/org/apache/nifi/prioritizer/PriorityAttributePrioritizer.java#L49-L57

  was:
The documentation for the compare() method of the Comparator interface\[1] states:

{quote}
The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y.
{quote}

However, in the PriorityAttributePrioritizer\[2], we have the following snippet:

{code}
String o1Priority = o1.getAttribute(CoreAttributes.PRIORITY.key());
String o2Priority = o2.getAttribute(CoreAttributes.PRIORITY.key());
if (o1Priority == null && o2Priority == null) {
return -1; // this is not 0 to match FirstInFirstOut
} else if (o2Priority == null) {
return -1;
} else if (o1Priority == null) {
return 1;
}
{code}

This implies that for two non-null FlowFiles f1 and f2, both with null "priority" attributes, we would have:

{code}
compare(f1, f2) == -1
compare(f2, f1) == -1
{code}

This would appear to violate the contract of the Comparator interface.  The comment suggests this was done to preserve FIFO ordering, however a return of 0 should do the same, and satisfy the contract.

\[1] https://docs.oracle.com/javase/7/docs/api/java/util/Comparator.html#compare%28T,%20T%29
\[2] https://github.com/apache/nifi/blob/d838f61291d2582592754a37314911b701c6891b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-prioritizers/src/main/java/org/apache/nifi/prioritizer/PriorityAttributePrioritizer.java#L49-L57


> PriorityAttributePrioritizer violates Comparator contract
> ---------------------------------------------------------
>
>                 Key: NIFI-2979
>                 URL: https://issues.apache.org/jira/browse/NIFI-2979
>             Project: Apache NiFi
>          Issue Type: Bug
>            Reporter: Brandon DeVries
>
> The documentation for the compare() method of the Comparator interface\[1] states:
> {quote}
> The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y.
> {quote}
> However, in the PriorityAttributePrioritizer\[2], we have the following snippet:
> {code}
> String o1Priority = o1.getAttribute(CoreAttributes.PRIORITY.key());
> String o2Priority = o2.getAttribute(CoreAttributes.PRIORITY.key());
> if (o1Priority == null && o2Priority == null) {
>      return -1; // this is not 0 to match FirstInFirstOut
> } else if (o2Priority == null) {
>      return -1;
> } else if (o1Priority == null) {
>      return 1;
> }
> {code}
> This implies that for two non-null FlowFiles f1 and f2, both with null "priority" attributes, we would have:
> {code}
> compare(f1, f2) == -1
> compare(f2, f1) == -1
> {code}
> This would appear to violate the contract of the Comparator interface.  The comment suggests this was done to preserve FIFO ordering, however a return of 0 should do the same, and satisfy the contract.
> \[1] https://docs.oracle.com/javase/7/docs/api/java/util/Comparator.html#compare%28T,%20T%29
> \[2] https://github.com/apache/nifi/blob/d838f61291d2582592754a37314911b701c6891b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-prioritizers/src/main/java/org/apache/nifi/prioritizer/PriorityAttributePrioritizer.java#L49-L57



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