Github user gabrielreid commented on the pull request:
https://github.com/apache/phoenix/pull/47#issuecomment-85666137
The tests you've added look good. It looks like the reason that you're
getting issues with the DESC tests is because you're returning the SortOrder of
the underlying column in the getSortOrder method. Just removing the
getSortOrder method completely should resolve this.
It looks like the filter tests also won't work. This will be because the
function instance isn't correctly being deserialized/initialized on the server
side. Adding in the following method will ensure that the initialization
happens correctly:
@Override
public void readFields(DataInput input) throws IOException {
super.readFields(input);
init();
}
I also took a closer look at the actual testing of the string searching
functionality. It looks to me like this could be a lot more simple, although
maybe I'm missing something. However, wouldn't it work to just do the following
in the evaluate method:
@Override
public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
if (!getStringExpression().evaluate(tuple, ptr)) {
return false;
}
String sourceStr = (String) PVarchar.INSTANCE.toObject(ptr,
getStringExpression().getSortOrder());
int position = sourceStr.indexOf(strToSearch);
ptr.set(PInteger.INSTANCE.toBytes(position));
return true;
}
Could you see if that works ok for all test cases, etc? It looks like the
code in the `searchString` method is doing a lot of unnecessary array copying
otherwise.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
