This repository has been archived by the owner on Aug 13, 2019. It is now read-only.
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.
Optimize queries using regex matchers for set lookups #602
Optimize queries using regex matchers for set lookups #602
Changes from 17 commits
d249d1d
32080de
3f60bf8
02dfa44
842e2a4
1e3cf63
4a66e34
87c3186
017871f
b81e86b
0f88eed
81cb028
954df56
c929d7a
959158e
591ae7c
2749e56
d30f3a2
6049a19
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this check necessary?
Why not just allow every escaped character? (Or perhaps disallow
\0...
since that’s complicated).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brian-brazil Should I include the cases above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, though check what Go does for escaping characters that don't need to be escaped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I was thinking about the wrong language.
The set of escapes we care about here is documented at https://github.com/google/re2/wiki/Syntax and is painfully complicated.
There are lots of escapes we don't want to accept, like
\g
,\p
,\x
, so you do need something like what you have.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have rechecked, the special characters you meant like
\n
,\a
are actually already included in theelse {}
part offindSetMatches
. What I detect here are the special characters like\\.
,\\+
in regexp, which means after I find\\
, I determine if the next char is special.