Skip to content

fix(bigquery-jdbc): Add escape character support for pattern matching#13259

Open
logachev wants to merge 2 commits into
mainfrom
kirl/patterns
Open

fix(bigquery-jdbc): Add escape character support for pattern matching#13259
logachev wants to merge 2 commits into
mainfrom
kirl/patterns

Conversation

@logachev
Copy link
Copy Markdown
Contributor

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\_table is not matching test_table.

@logachev logachev requested review from a team as code owners May 22, 2026 20:56
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

@logachev
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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('$');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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('$');

Comment on lines +255 to +256
assertTrue(escapedPercent.matcher("100%discount").matches());
assertFalse(escapedPercent.matcher("100PERCENTdiscount").matches());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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
  1. Before adding new test cases for input validation, verify the existing test suite to ensure these scenarios are not already covered.

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.

1 participant