spark-test commented on code in PR #56558:
URL: https://github.com/apache/spark/pull/56558#discussion_r4480202248
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/util/V2ExpressionSQLBuilder.java:
##########
@@ -90,9 +90,9 @@ protected String escapeSpecialCharsForLikePattern(String str)
{
* The default returns the input unchanged: a standard SQL string literal is
taken verbatim (the
* single-quote doubling is already applied when the literal is rendered),
so the {@code \} that
* {@link #escapeSpecialCharsForLikePattern} uses as the LIKE escape
character reaches the LIKE
- * engine intact. A dialect whose string-literal syntax gives {@code \} a
special meaning (e.g.
- * MySQL, which treats {@code \} as an escape character inside string
literals) must override this
- * to double the backslash, so the LIKE pattern survives string-literal
parsing unchanged.
+ * engine intact. A dialect whose string-literal syntax gives {@code \} a
special meaning must
Review Comment:
**Nit (P3):** With this PR, Spark's JDBC dialects no longer go through this
hook. `JDBCSQLBuilder.build` handles `STARTS_WITH`/`ENDS_WITH`/`CONTAINS`
before `super.build`, escapes the raw pattern with `likeEscapeChar`, and
renders the pattern and the `ESCAPE` character through
`JdbcDialect.compileValue`. So `likeWithEscape`, the only caller of
`escapeStringLiteralForLikePattern`, is never reached for them. This sentence
still tells such a dialect to override the hook and double backslashes. A
dialect on the JDBC builder that follows it would see no effect, and doubling
is the `sql_mode`-dependent approach this PR just replaced for MySQL. The note
on `escapeSpecialCharsForLikePattern` (lines 64-68) has the same problem.
Could both javadocs say that these hooks serve this class's own
`visitStartsWith`/`visitEndsWith`/`visitContains`, which direct subclasses
still use? They should also say that Spark's JDBC dialects add string-literal
escaping in `compileValue` instead, and limit the doubling advice to engines
whose string literals always treat a backslash as an escape.
**Verification:**
- **Inspection:** Check that the revised javadocs describe the mechanism the
JDBC LIKE-family builder actually uses (a dialect escape character, with the
pattern and the escape character rendered through compileValue). Check that
they keep the hook guidance only for the base visitors that still call it.
- **Compile:** Check that the Java API documentation for the edited class
still builds without broken references from the revised comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
