Fix sql LIKE usage to properly escape special characters #6837
Conversation
| { | ||
| String prefix = wildcardPrefix != null ? wildcardPrefix : ""; | ||
| String suffix = wildcardSuffix != null ? wildcardSuffix : ""; | ||
| String prefixLike = prefix + CompareType.escapeLikePattern(matchStr, escapeChar) + suffix; |
There was a problem hiding this comment.
Should this be using concatenate() as furnished by the dialect?
There was a problem hiding this comment.
If matchStr were not constant (e.g. SQLFragment) then yes, but since these are all constant strings, this makes more sense.
| return sql; | ||
| } | ||
|
|
||
| public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr, @Nullable String wildcardPrefix, @Nullable String wildcardSuffix) |
There was a problem hiding this comment.
I recommend unit tests for each of these incantations.
| @@ -535,6 +536,40 @@ public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection<?> par | |||
| return DEFAULT_GENERATOR.appendInClauseSql(sql, params); | |||
| } | |||
|
|
|||
| public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr, @Nullable String wildcardPrefix, @Nullable String wildcardSuffix, char escapeChar) | |||
There was a problem hiding this comment.
nit: It would be nice to get ConvertType.QClause.toSQLFragment() converted over to using this.
| .append(getCaseInsensitiveLikeOperator()) | ||
| .append(" ") | ||
| .appendValue(prefixLike) | ||
| .append(escapeToken); |
There was a problem hiding this comment.
I think escapeToken is always !, but it would be better to use quoteStringLiteral()
There was a problem hiding this comment.
For sql server, quoteStringLiteral adds a "N" prefix and that doesn't work for the escape character.
Rationale
This has been an ongoing intermittent TC failure on sql server that fails to correctly increment withCounter sequences, when the string contains special characters. Wildcard characters need to be escaped for LIKE operators in SQL.
Related Pull Requests
Changes
Tasks 📍
~~- [ ] Manual Testing ~~
Needs Automation (No, already exist)