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

James Taylor commented on PHOENIX-1002:
---------------------------------------

Looking good, thanks for making the changes, [~kbuzsaki]. One question, back on 
the precedence issue: can you add a test that uses only + and - and depends on 
the operators being evaluated left to right?

Couple of minor/subtle issues for the evaluate method that I missed before. 
Only return false if the child expression returns false. If the expression 
itself evaluates to null (i.e. ptr.getLength() == 0), then return true. The 
difference is subtle and really only matters when evaluated on the server-side, 
where we're seeing KeyValue by KeyValue. When we can't evaluate an expression 
because we haven't yet seen the KeyValue we need, we return false (meaning that 
we don't have enough information yet to evaluate the entire expression). If we 
return true, it means we've been able to evaluate it (whether it evaluates to 
null or not).

Not positive on what should happen if divisor is zero too. If we should raise 
an exception or return true with ptr pointing to an empty byte array. Seems ok 
to return null for now, but might check (you probably already did?) on what we 
do for divide by zero.

{code}
+    @Override
+    public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
+        // get the dividend
+        Expression dividendExpression = getDividendExpression();
+        if (!dividendExpression.evaluate(tuple, ptr)) {
+            return false;
+        }
+        if (ptr.getLength() == 0) {
+            return false;
+        }
+        long dividend = 
dividendExpression.getDataType().getCodec().decodeLong(ptr, 
dividendExpression.getSortOrder());
+        
+        // get the divisor
+        Expression divisorExpression = getDivisorExpression();
+        if (!divisorExpression.evaluate(tuple, ptr)) {
+            return false;
+        }
+        if (ptr.getLength() == 0) {
+            return false;
+        }
+        long divisor = 
divisorExpression.getDataType().getCodec().decodeLong(ptr, 
divisorExpression.getSortOrder());
+        // cannot divide by zero
+        if(divisor == 0) { 
+            return false;
+        }
+        
+        // actually perform modulus
+        long remainder = dividend % divisor;
+        
+        ptr.set(PDataType.LONG.toBytes(remainder));
+        return true;
+    }
{code}


> Add support for % operator
> --------------------------
>
>                 Key: PHOENIX-1002
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1002
>             Project: Phoenix
>          Issue Type: New Feature
>            Reporter: Thomas D'Silva
>         Attachments: PHOENIX-1002.patch, PHOENIX-1002_2.patch, 
> PHOENIX-1002_3.patch, PHOENIX-1002_4.patch
>
>
> Supporting the % operator would allow using sequences to generate IDs that 
> are less than a maximum number. 
> CREATE SEQUENCE foo.bar
> SELECT ((NEXT VALUE FOR foo.bar)%1000)



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to