This repository has been archived by the owner on Jan 28, 2021. It is now read-only.
Convert LIKE patterns to specific Go regexes #817
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #814
The problem here was that we converted the
LIKE
pattern into a regex and used one of the regex engines (the standard go regexp package, or oniguruma) to try to match the string from the input row. The regex was built by enclosing theLIKE
pattern between^
and$
(so if the pattern wastext
, the regexp would be^text$
). But, at least in the standard go regexp package,^
and$
match the beginning and the end of the text, not of the line, and the dot.
matches any character that is not a new line character. That is, if theLIKE
pattern is%text%
, then the resulting regex,^.*text.*$
, will match anything that contains the substring text and that does not contain any new line characters. I considered several solutions to fix this:internal/regex_go.go
, either by:(?s)
, which makes.
match also newline characters(?m)
, which makes^
and$
match beginning and end, respectively, of a new line, not of the whole text.patternToRegex
function insql/expression/like.go
so it depends on the regex engine being used.patternToRegex
specific to the go engine, always adding the(?s)
flag.I finally implemented the last solution, as it was the least intrusive. The other ones are not generic or mix two different problems in one single place.
Note: We should block the merge of this PR until @Hydrocharged opens one with the tests.