Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LIKE implementation #476 #512

Merged
merged 6 commits into from Aug 26, 2020
Merged

LIKE implementation #476 #512

merged 6 commits into from Aug 26, 2020

Conversation

mick2004
Copy link
Contributor

Tested via webUI and automated tests.
Screenshot 2020-07-19 at 10 45 44 PM
Screenshot 2020-07-19 at 10 45 51 PM
Screenshot 2020-07-19 at 10 45 59 PM
Screenshot 2020-07-19 at 10 45 32 PM

@bluestreak01
Copy link
Member

Thank you Abhishek!

Please have a look here on how “like” is expected to work:

https://www.w3schools.com/sql/trysql.asp?filename=trysql_select_like

@mick2004
Copy link
Contributor Author

@bluestreak01 sure

@bluestreak01
Copy link
Member

cool, thanks. I just wanted to draw your attention to the fact that 'like' is a pattern matching operator and like 'a' should only match exact a strings

@mick2004
Copy link
Contributor Author

mick2004 commented Jul 20, 2020

cool, thanks. I just wanted to draw your attention to the fact that 'like' is a pattern matching operator and like 'a' should only match exact a strings

@bluestreak01 Understood.So are below criteria ok for LIKE or I am missing anything ?

1.like '<>' return exact match of <>
2.like '<>%' return records starting with <>
3.like '%<>' return records ending with <>
4.like '%<>%' return match of <> anywhere.This is equivalent of ~= behaviour

@bluestreak01
Copy link
Member

bluestreak01 commented Jul 20, 2020

There is also underscore (_) - matches any one character. “Like” is best described here: https://www.w3schools.com/sql/sql_like.asp

% is a wildcard, it can be present in the pattern 0 or more types, more than 2 certainly. One way to deal with this would be rewriting “like” pattern to a regex and executing the latter

@mick2004
Copy link
Contributor Author

There is also underscore (_) - matches any one character. “Like” is best described here: https://www.w3schools.com/sql/sql_like.asp

% is a wildcard, it can be present in the pattern 0 or more types, more than 2 certainly. One way to deal with this would be rewriting “like” pattern to a regex and executing the latter

Sure,let me check in more depth.

@mick2004
Copy link
Contributor Author

mick2004 commented Jul 21, 2020

@bluestreak01 Pushed changes for:
1.Handling %
2.Handling _
3.Handling Exact match
4.Skipping special characters

Please review.

@Override
public boolean getBool(Record rec) {
CharSequence cs = getArg().getStr(rec);
return cs != null && cs.length()==1 && Character.toLowerCase(cs.charAt(0))==Character.toLowerCase(expected);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL 'like' is case sensitive

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluestreak01 https://www.w3schools.com/sql/trysql.asp?filename=trysql_select_like

Try query : SELECT * FROM Customers WHERE CustomerName LIKE 'alfreds%';

I am getting results for Alfred. Just want to double check before making change.

CharSequence likeString=args.getQuick(1).getStr(null);

if (likeString == null) {
throw SqlException.$(args.getQuick(1).getPosition(), "NULL likeString");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message is shown to user as well as position of the error. User needs to know what to do with this. 'likeString' is not a commonly known term :)

Copy link
Contributor Author

@mick2004 mick2004 Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluestreak01 What should be message. Is "NULL not supported for like expression" ok ? else suggest some message

return value;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What IDE are you using? Would it be possible to format the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return "";
}

StringBuilder sb = new StringBuilder(s.length() * 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringBuilder is transient and throw-away instance. Please instead use

Misc.getThreadLocalBuilder()

}

String regex = escapeSpecialChars(likeString).
replace('_', '.').replace("%", ".*?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be done in the same loop as escaping special characters, which would eliminate two extra loops and two string instances.


try {
Matcher matcher = Pattern.compile(regex,
Pattern.CASE_INSENSITIVE | Pattern.DOTALL).matcher("");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'like' is case sensitive

}
sb.append(c);
}
return sb.toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use Chars.toString(). Instances of strings are liable to be cached in the future.

int len = s.length();
if (len == 0)
{
return "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no unit test that hits this line



} catch (PatternSyntaxException e) {
throw SqlException.$(args.getQuick(1).getPosition() + e.getIndex() + 1, e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is untested

@@ -54,6 +54,7 @@
add(new OperatorExpression("and", 11, true, BINARY, false));
add(new OperatorExpression("or", 11, true, BINARY, false));
add(new OperatorExpression("not", 11, true, UNARY, false));
add(new OperatorExpression("like", 7, true, BINARY));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'like' must not be a symbol (see 'and', 'not' etc.)

The reason is that symbols break up words, for example 'dislike' will be tokenized as 'dis' + 'like' if 'like' was a symbol. When 'like' is not a symbol - 'dislike' will remain whole and 'like' will only be tokenized as such when it is separated with whitespace, e.g. x like '%a'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluestreak01 understood

@CLAassistant
Copy link

CLAassistant commented Aug 19, 2020

CLA assistant check
All committers have signed the CLA.

@mick2004
Copy link
Contributor Author

@bluestreak01 Can you please review again.Pushed changes as per your feedback.

@bluestreak01 bluestreak01 merged commit 0cae67f into questdb:master Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants