[
https://issues.apache.org/jira/browse/PHOENIX-4884?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16604759#comment-16604759
]
James Taylor commented on PHOENIX-4884:
---------------------------------------
Here's some feedback on your second patch, [~elserj]:
No need to set ptr to EMPTY_ARRAY since it's already set to this. It should
look something like this instead:
{code:java}
+ if (sourceStr == null) {
+ Expression child = getChildren().get(0);
+
+ if (!child.evaluate(tuple, ptr)) {
+ return false;
+ }
+
+ // We need something non-empty to search against
+ if (ptr.getLength() == 0) {
+ return true;
+ }
+
+ sourceStr = (String) PVarchar.INSTANCE.toObject(ptr,
child.getSortOrder());
}
{code}
The second if statement should mimic the above and look like this:
{code:java}
+ String searchStr = literalSearchStr;
+ // 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) {
+ return true;
+ }
+
+ searchStr = (String) PVarchar.INSTANCE.toObject(ptr,
child.getSortOrder());
}
{code}
This test is good, but you should always include a test for a built-in function
in which it's called in a WHERE clause so that you test the server-side
execution as well (SELECT clause is executed on the client-side):
{code:java}
+ @Test
+ public void testNonLiteralSourceExpression() throws Exception {
+ Connection conn = DriverManager.getConnection(getUrl());
+ String tableName = generateUniqueName();
+ initTable(conn, tableName, "ASC", "asdf", "sdf");
+ String query = "SELECT INSTR('asdf', 'sdf') FROM " + tableName;
+ testInstr(conn, query, 2);
+ query = "SELECT INSTR('asdf', substr) FROM " + tableName;
+ testInstr(conn, query, 2);
+ query = "SELECT INSTR('qwerty', 'sdf') FROM " + tableName;
+ testInstr(conn, query, 0);
+ query = "SELECT INSTR('qwerty', substr) FROM " + tableName;
+ testInstr(conn, query, 0);
+ }
{code}
> 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
>
>
> 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)
