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

Josh Elser commented on PHOENIX-4884:
-------------------------------------

Attached an .003. Incorporated the feedback, except for one:
{code:java}
+        // A literal was not provided, try to evaluate the expression to a 
literal
+        if (searchStr == null){
+            Expression child = getChildren().get(1);
+
+            if (!child.evaluate(tuple, ptr)) {
+              return false;
+            }
+
+            // A null (or zero-length) search string
+            if (ptr.getLength() == 0) {
+              ptr.set(PInteger.INSTANCE.toBytes(0));
+              return true;
+            }
+
+            searchStr = (String) PVarchar.INSTANCE.toObject(ptr, 
child.getSortOrder());{code}
If I don't set {{ptr}} to {{0}}, this causes a NPE in 
{{InstrFunctionTest.java}} when the search string is the empty string. I think 
setting the result to be {{0}} is the right thing to do (if the user gives us 
an empty string to search for, we always know that the result should be {{0}}), 
but I may be ignorant of how we should correctly be computing that.

> INSTR function should work seamlessly with literal and non-literal arguments
> ----------------------------------------------------------------------------
>
>                 Key: PHOENIX-4884
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4884
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Josh Elser
>            Assignee: Josh Elser
>            Priority: Major
>             Fix For: 4.15.0, 5.1.0
>
>         Attachments: PHOENIX-4884.001.patch, PHOENIX-4884.002.patch, 
> PHOENIX-4884.003.patch
>
>
> INSTR's documentation reads as though it should support an expression or a 
> literal for either argument. At least, it doesn't say that it only supports 
> one or the other.
> However, the implementation only handles the case of {{INSTR(expr, 
> literal)}}. We can pretty easily make this better and work with any 
> combination:
> e.g. {{INSTR(literal, expr)}}, {{INSTR(expr, expr)}}, {{INSTR(literal, 
> literal)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to