[ 
https://issues.apache.org/jira/browse/TAJO-182?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13855134#comment-13855134
 ] 

Jihoon Son commented on TAJO-182:
---------------------------------

The patch is hard to review, because it includes multiple issues.
Anyway, in overall, the patch looks good to me.
I suggest two things as follows.
* In Inet4Datum.compareTo(), you look to miss when two datums are equal. A code 
should be added below the for clause as follows
{code}
    case INET4:
      byte [] bytes = asByteArray();
      byte [] other = datum.asByteArray();
      
      for (int i = 0; i < 4; i++) {
        if (bytes[i] > other[i]) {
          return 1;
        } else if (bytes[i] < other[i]) {
          return -1;
        }
      }
      return 0; // should be added
{code}
* How about define integer values to represent three-values? I think that it 
would increase the readability. Here is an example.
*Current*
{code}
  public Datum and(Datum datum) {
    return BooleanDatum.AND_LOGIC[0][datum.asInt4()];
  }
{code}
*Suggestion*
{code}
  public Datum and(Datum datum) {
    return BooleanDatum.AND_LOGIC[UNKNOWN_INTEGER][datum.asInt4()];
  }
{code}

> Comparison of primitive values including null value should return NULL.
> -----------------------------------------------------------------------
>
>                 Key: TAJO-182
>                 URL: https://issues.apache.org/jira/browse/TAJO-182
>             Project: Tajo
>          Issue Type: Bug
>          Components: planner/optimizer
>            Reporter: Hyunsik Choi
>            Assignee: Hyunsik Choi
>            Priority: Critical
>             Fix For: 0.8-incubating
>
>         Attachments: TAJO-182.patch, TAJO-182_2.patch
>
>
> -If some domain value is compared to Null value, the current implementation 
> will cause InvalidOperationException. Such cases should result in 'false'.-
> If some domain value is compared to Null value, the current implementation 
> will cause either InvalidOperationException or FALSE. Such cases should 
> result in NULL.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to