fix(bigquery-jdbc): Add escape character support for pattern matching#13259
fix(bigquery-jdbc): Add escape character support for pattern matching#13259logachev wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the compileSqlLikePattern method in BigQueryDatabaseMetaData to correctly handle escaped SQL wildcards and regex metacharacters, ensuring that backslashes are treated as escape characters. It also includes new unit tests to verify the behavior of escaped underscores and percent signs. Review feedback suggests simplifying the pattern compilation logic to reduce code duplication and refactoring the isRegexMetacharacter helper method to be static and more concise.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the compileSqlLikePattern method in BigQueryDatabaseMetaData to support escaped SQL LIKE wildcards using a state-based parsing approach. It also introduces a helper method to identify regex metacharacters and adds unit tests for escaped underscores and percent signs. Feedback suggests addressing a bug where trailing backslashes are ignored and recommends adding more comprehensive test cases for escaped backslashes and trailing escape characters.
| escaped = false; | ||
| } | ||
| } | ||
| regex.append('$'); |
There was a problem hiding this comment.
The current implementation ignores a trailing backslash if it is the last character in the sqlLikePattern. Previously, a trailing backslash was treated as a literal character and escaped in the resulting regex. To maintain backward compatibility and handle this edge case, the code should check if the escaped flag is still true after the loop and append the escaped backslash to the regex.
if (escaped) {
regex.append('\\').append('\\');
}
regex.append('$');| assertTrue(escapedPercent.matcher("100%discount").matches()); | ||
| assertFalse(escapedPercent.matcher("100PERCENTdiscount").matches()); |
There was a problem hiding this comment.
Consider adding test cases for an escaped escape character (e.g., \\) and a trailing escape character to ensure they are handled correctly and to prevent future regressions. Before adding these, please verify the existing test suite to ensure these scenarios are not already covered.
assertTrue(escapedPercent.matcher("100%discount").matches());
assertFalse(escapedPercent.matcher("100PERCENTdiscount").matches());
// Escaped escape character
Pattern escapedBackslash = dbMetadata.compileSqlLikePattern("test\\\\table");
assertTrue(escapedBackslash.matcher("test\\table").matches());
// Trailing escape character
Pattern trailingBackslash = dbMetadata.compileSqlLikePattern("test\\");
assertTrue(trailingBackslash.matcher("test\\").matches());References
- Before adding new test cases for input validation, verify the existing test suite to ensure these scenarios are not already covered.
Various metadata methods (
getColumns,getTables) rely on pattern matching. This PR adds a support for the escape character in these pattern matches.Example of issue:
test\_tableis not matchingtest_table.