[SPARK-57446][SQL] Apply `escapeSql` to `comment` in JDBC `table/schema` comment queries by dongjoon-hyun · Pull Request #56506 · apache/spark

dongjoon-hyun · 2026-06-14T19:35:41Z

What changes were proposed in this pull request?

This PR applies escapeSql to the comment argument of the JDBC dialect comment query
methods: JdbcDialect.getTableCommentQuery, JdbcDialect.getSchemaCommentQuery, and the
MySQLDialect.getTableCommentQuery override.

Why are the changes needed?

The comment was embedded into a single-quoted SQL literal without escaping, so a comment
containing ' breaks the statement. escapeSql doubles single quotes (' -> ''):

Before: COMMENT ON TABLE t IS 'a'b' (invalid)
After: COMMENT ON TABLE t IS 'a''b' (correct)

This is consistent with the existing string value treatment.

spark/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala

Lines 384 to 385 in 875982c

    
           def compileValue(value: Any): Any = value match { 
        
             case stringValue: String => s"'${escapeSql(stringValue)}'"

Does this PR introduce any user-facing change?

Yes. A table or schema comment containing a single quote is now stored correctly instead
of failing with a SQL syntax error.

How was this patch tested?

Pass the CIs with a newly added test case.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.8)

…mment queries

dongjoon-hyun · 2026-06-14T19:45:39Z

cc @huaxingao , @cloud-fan

cloud-fan

0 blocking, 1 non-blocking, 0 nits.
Clean, well-scoped fix. The escaping is consistent with the established compileValue pattern, covers every dialect override that embeds a comment literal (the others throw unsupported), and the new test genuinely exercises the escaping. LGTM.

Correctness (1)

MySQLDialect.scala:269: escapeSql doubles ' but doesn't escape \ (non-blocking, out of scope) — see inline

cloud-fan · 2026-06-15T01:19:26Z

  // See https://dev.mysql.com/doc/refman/8.0/en/alter-table.html
  override def getTableCommentQuery(table: String, comment: String): String = {
-    s"ALTER TABLE $table COMMENT = '$comment'"
+    s"ALTER TABLE $table COMMENT = '${escapeSql(comment)}'"


Non-blocking, out of scope — just flagging for completeness: escapeSql doubles ' but doesn't escape \. MySQL treats backslash as an escape character by default (NO_BACKSLASH_ESCAPES unset), so a comment containing a backslash can still be mangled (or break the statement if trailing) here. This is pre-existing and shared with compileValue's MySQL string-predicate handling, and this PR is a strict improvement for the single-quote case, so nothing to change here.

dongjoon-hyun · 2026-06-15T01:28:55Z

Thank you, @cloud-fan . Yes, it is.

…ma` comment queries ### What changes were proposed in this pull request? This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. ### Why are the changes needed? The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 ### Does this PR introduce _any_ user-facing change? Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. ### How was this patch tested? Pass the CIs with a newly added test case. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 072994d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

dongjoon-hyun · 2026-06-15T01:30:20Z

Merged to master/4.x for now because we have Apache Spark 4.2.0 RC3 vote currently.

If it fails, I will consider to cherry-pick this to branch-4.2 and older.

…ma` comment queries ### What changes were proposed in this pull request? This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. ### Why are the changes needed? The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 ### Does this PR introduce _any_ user-facing change? Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. ### How was this patch tested? Pass the CIs with a newly added test case. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 072994d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 012a3d8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…ma` comment queries This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. Pass the CIs with a newly added test case. Generated-by: Claude Code (Claude Opus 4.8) Closes #56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 072994d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 012a3d8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit bf50ad9) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…ma` comment queries This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. Pass the CIs with a newly added test case. Generated-by: Claude Code (Claude Opus 4.8) Closes #56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 072994d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 012a3d8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit bf50ad9) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 4c57f43) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…ma` comment queries This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. Pass the CIs with a newly added test case. Generated-by: Claude Code (Claude Opus 4.8) Closes #56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 072994d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 012a3d8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit bf50ad9) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 4c57f43) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 748f602) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…ma` comment queries ### What changes were proposed in this pull request? This PR applies `escapeSql` to the `comment` argument of the JDBC dialect comment query methods: `JdbcDialect.getTableCommentQuery`, `JdbcDialect.getSchemaCommentQuery`, and the `MySQLDialect.getTableCommentQuery` override. ### Why are the changes needed? The `comment` was embedded into a single-quoted SQL literal without escaping, so a comment containing `'` breaks the statement. `escapeSql` doubles single quotes (`'` -> `''`): - Before: `COMMENT ON TABLE t IS 'a'b'` (invalid) - After: `COMMENT ON TABLE t IS 'a''b'` (correct) This is consistent with the existing string value treatment. https://github.com/apache/spark/blob/875982c4a65bc703bce88fb17683b423d8f88255/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala#L384-L385 ### Does this PR introduce _any_ user-facing change? Yes. A table or schema comment containing a single quote is now stored correctly instead of failing with a SQL syntax error. ### How was this patch tested? Pass the CIs with a newly added test case. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes apache#56506 from dongjoon-hyun/SPARK-57446. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

[SPARK-57446][SQL] Apply escapeSql to comment in JDBC table/schema co…

e09bb9a

…mment queries

cloud-fan approved these changes Jun 15, 2026

View reviewed changes

dongjoon-hyun closed this in 072994d Jun 15, 2026

dongjoon-hyun deleted the SPARK-57446 branch June 15, 2026 01:30

dongjoon-hyun mentioned this pull request Jun 16, 2026

[SPARK-57496][SQL][BUILD] Keep the Types Framework ops and UDF worker packages out of the published API #56551

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-57446][SQL] Apply `escapeSql` to `comment` in JDBC `table/schema` comment queries#56506

[SPARK-57446][SQL] Apply `escapeSql` to `comment` in JDBC `table/schema` comment queries#56506
dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-57446

dongjoon-hyun commented Jun 14, 2026 •

edited

Loading

Uh oh!

dongjoon-hyun commented Jun 14, 2026

Uh oh!

cloud-fan left a comment

Uh oh!

cloud-fan Jun 15, 2026

Uh oh!

dongjoon-hyun commented Jun 15, 2026

Uh oh!

dongjoon-hyun commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def compileValue(value: Any): Any = value match {
	case stringValue: String => s"'${escapeSql(stringValue)}'"

Uh oh!

Conversation

dongjoon-hyun commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

dongjoon-hyun commented Jun 14, 2026

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Correctness (1)

Uh oh!

cloud-fan Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Jun 15, 2026

Uh oh!

dongjoon-hyun commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dongjoon-hyun commented Jun 14, 2026 •

edited

Loading