[ItsDeadlyProgrammer]

Follow-up to #1936.

The dialects that escape a quote with a backslash (MySQL, MariaDB, BigQuery, Spark SQL, ClickHouse, Infobright) also treat a lone backslash as an escape character, but a backslash in a value was copied as is. A value like C:\temp\ came out as

... VALUES ('C:\temp\');

where the last backslash escapes the closing quote, so MySQL rejects the statement (and \t turns into a tab).

Now a backslash is doubled whenever the quote is escaped with a backslash, which gives 'C:\\temp\\'. That includes the no-dialect output, since it already uses \'. Dialects that double quotes (PostgreSQL, Oracle, SQL Server, H2, ...) don't treat backslash specially and are unchanged.

Tests: escapesBackslashInStringValues (per dialect) and escapesBackslashBeforeQuote (a\'b becomes 'a\\\'b'). Both fail without the change. SqlTest passes (109 tests).

[what-the-diff]

PR Summary

• Improved SQL string escaping for affected dialects
  Literal backslashes are now escaped when a SQL dialect uses backslashes to escape quotes, helping prevent string values from being interpreted incorrectly.

• Added cross-dialect test coverage
  Tests verify backslash handling, including when a backslash appears immediately before a quote.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.41%. Comparing base (471b259) to head (c6e2e8b).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1939      +/-   ##
============================================
+ Coverage     92.40%   92.41%   +0.01%     
- Complexity     3565     3570       +5     
============================================
  Files           348      348              
  Lines          7055     7065      +10     
  Branches        692      691       -1     
============================================
+ Hits           6519     6529      +10     
- Misses          370      371       +1     
+ Partials        166      165       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[asolntsev]

I got a feeling that this a "hack for a hack".
Shouldn't every dialect know explicitly how to escape values?

[ItsDeadlyProgrammer]

Fair point. Right now the dialect only answers a yes/no question (escapesQuoteWithBackslash()), and SqlTransformer works out the actual escaping from that. That's why the backslash case reads like a patch on top of a patch.

What do you think of moving it into the dialect itself, something like:

// SqlDialect
public String escapeStringLiteral(String value) {
    return switch (this) {
        case BIGQUERY, CLICKHOUSE, INFOBRIGHT, MARIADB, MYSQL, SPARKSQL ->
            value.replace("\\", "\\\\").replace("'", "\\'");
        default -> value.replace("'", "''");
    };
}

Then SqlTransformer just calls dialect.escapeStringLiteral(value) and doesn't need to know about quote or backslash rules at all. If a dialect needs something special later, it only changes in one place. escapesQuoteWithBackslash() and the quoteEscape field can go away.

If that sounds good I'll rework the PR that way. I can do the same for identifiers in #1940 (escapeIdentifier) so both follow one pattern.

[asolntsev]

Yes, this sounds good.
But even better is to avoid the "switch" at all - just pass the needed parameters to Dialect constructor.

[ItsDeadlyProgrammer]

Done. Each dialect now gets its escaping rule through the constructor (MYSQL("`", Casing.UNCHANGED, BACKSLASH_ESCAPE) etc.), and SqlDialect.escapeStringLiteral(value, quote) uses it, no switch. SqlTransformer only calls the dialect now, and escapesQuoteWithBackslash() is gone (it wasn't released yet).

[asolntsev]

Thanks for the PR! A few remarks inline.

One more place outside the diff: SqlTransformer.addValues() (around line 118) still wraps the SPARKSQL field prefix as quote + dialect.getFieldPrefix(...) + quote without escaping. A nested field named it's or a\b produces named_struct('it's', ...), which is invalid SQL. It should go through dialect.escapeStringLiteral(...) too.

[ItsDeadlyProgrammer]

Thanks, pushed 23c567d:

• The SPARKSQL field prefix now goes through the same escaping (it's and a\b are covered by a test).
• escapesQuoteWithBackslash() is back, marked @Deprecated.
• The boolean flag is replaced by a small QuoteEscape enum (DOUBLE / BACKSLASH).
• Without a dialect the behaviour is unchanged from before this PR: only the quote gets a backslash, backslashes are left alone. Test added.
• SNOWFLAKE is now BACKSLASH too. CLICKHOUSE, INFOBRIGHT and SNOWFLAKE are in the dialect tests.
• The NO_BACKSLASH_ESCAPES caveat is documented on escapeStringLiteral.

#1940 is rebased on top.

[asolntsev]

Thanks, all points are addressed. Just two small things left inline.

[ItsDeadlyProgrammer]

Done, pushed: removed escapesQuoteWithBackslash() and its test, and fixed the javadoc example to 'It\'s'. SqlTest passes.

[asolntsev]

Thanks for addressing all the comments! LGTM.