-
Notifications
You must be signed in to change notification settings - Fork 131
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
Fix panic of empty value in filter config #799
Conversation
check not having empty value now
re = regexp.MustCompile(fmt.Sprintf("(?i)%s", originStr[1:])) | ||
} else { // must match completely | ||
re = regexp.MustCompile(fmt.Sprintf("(?i)^%s$", originStr)) |
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.
If originStr
is empty, the regex become ^$
?
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.
yes, the else case means you must match totally.
but we check the config for empty schema and table at startup time, so it will not run into this case now actually, because it's meaningless for an empty schema and table, may cause by misconfiguring.
/run-all-tests |
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.
LGTM
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.
LGTM
cherry pick to release-3.1 in PR #800 |
cherry pick to release-2.1 failed |
cherry pick to release-3.0 in PR #801 |
Fix panic of empty value in filter config Verify config about filters with empty schema or table name.
What problem does this PR solve?
Fix panic of empty value in filter config
Verify config about filters
What is changed and how it works?
Fix panic of empty value in filter config
Verify config about filters
Check List
Tests
Code changes
Side effects
Related changes
Need to cherry-pick to the release branch
Need to be included in the release note