This is an automated email from the ASF dual-hosted git repository.
j1wonpark pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/amoro.git
The following commit(s) were added to refs/heads/master by this push:
new 6d3cd428c [AMORO-4423][Common] Escape Spark SQL literals in
table-operation history (#4424)
6d3cd428c is described below
commit 6d3cd428c1c7d6c2f16ca9b3ac2d37171c580480
Author: hutiefang76 <[email protected]>
AuthorDate: Fri Oct 16 19:21:09 2026 -0700
[AMORO-4423][Common] Escape Spark SQL literals in table-operation history
(#4424)
Escape Spark SQL literals in table-operation history
Co-authored-by: Jiwon Park <[email protected]>
---
.../descriptor/SparkMetadataChangeHandler.java | 28 +++-
.../TestSparkMetadataChangeHandlerLiterals.java | 186 +++++++++++++++++++++
2 files changed, 211 insertions(+), 3 deletions(-)
diff --git
a/amoro-common/src/main/java/org/apache/amoro/table/descriptor/SparkMetadataChangeHandler.java
b/amoro-common/src/main/java/org/apache/amoro/table/descriptor/SparkMetadataChangeHandler.java
index fae10d578..16f8157de 100644
---
a/amoro-common/src/main/java/org/apache/amoro/table/descriptor/SparkMetadataChangeHandler.java
+++
b/amoro-common/src/main/java/org/apache/amoro/table/descriptor/SparkMetadataChangeHandler.java
@@ -39,6 +39,20 @@ public class SparkMetadataChangeHandler implements
MetadataChangeHandler {
private final String tableName;
+ /**
+ * Escapes a raw value so it can be spliced between single quotes in a Spark
SQL literal. Spark
+ * uses backslash escapes rather than the SQL-standard doubled apostrophe,
so backslashes are
+ * doubled first and apostrophes escaped second.
+ */
+ private static String escapeLiteral(String value) {
+ return value
+ .replace("\\", "\\\\")
+ .replace("'", "\\'")
+ .replace("\n", "\\n")
+ .replace("\r", "\\r")
+ .replace("\t", "\\t");
+ }
+
public SparkMetadataChangeHandler(String tableName) {
this.tableName = tableName;
}
@@ -48,7 +62,11 @@ public class SparkMetadataChangeHandler implements
MetadataChangeHandler {
String template = ALTER_TABLE + SET_PROPERTIES;
String properties =
diffProperties.entrySet().stream()
- .map(entry -> String.format("'%s' = '%s'", entry.getKey(),
entry.getValue()))
+ .map(
+ entry ->
+ String.format(
+ "'%s' = '%s'",
+ escapeLiteral(entry.getKey()),
escapeLiteral(entry.getValue())))
.collect(Collectors.joining(","));
return String.format(template, tableName, properties);
}
@@ -57,7 +75,9 @@ public class SparkMetadataChangeHandler implements
MetadataChangeHandler {
public String removeProperties(Set<String> removeKeys) {
String template = ALTER_TABLE + UNSET_PROPERTIES;
String properties =
- removeKeys.stream().map(key -> String.format("'%s'",
key)).collect(Collectors.joining(","));
+ removeKeys.stream()
+ .map(key -> String.format("'%s'", escapeLiteral(key)))
+ .collect(Collectors.joining(","));
return String.format(template, tableName, properties);
}
@@ -93,7 +113,9 @@ public class SparkMetadataChangeHandler implements
MetadataChangeHandler {
@Override
public String changeColumnsComment(String columnName, String comment) {
String template = ALTER_TABLE + ALTER_COLUMN + DOC;
- return String.format(template, tableName, columnName, comment);
+ // A null comment means the comment is being removed, which Spark spells
as an empty literal.
+ return String.format(
+ template, tableName, columnName, escapeLiteral(comment == null ? "" :
comment));
}
@Override
diff --git
a/amoro-common/src/test/java/org/apache/amoro/table/descriptor/TestSparkMetadataChangeHandlerLiterals.java
b/amoro-common/src/test/java/org/apache/amoro/table/descriptor/TestSparkMetadataChangeHandlerLiterals.java
new file mode 100644
index 000000000..5ef3d1dd3
--- /dev/null
+++
b/amoro-common/src/test/java/org/apache/amoro/table/descriptor/TestSparkMetadataChangeHandlerLiterals.java
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.amoro.table.descriptor;
+
+import org.apache.amoro.table.TableIdentifier;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Guards single-quoted Spark SQL literal escaping. Spark uses backslash
escapes inside regular
+ * string literals, not the SQL-standard doubled apostrophe, so a raw
backslash must be doubled and
+ * a raw apostrophe backslash-escaped wherever a key, value or comment is
spliced between single
+ * quotes.
+ */
+public class TestSparkMetadataChangeHandlerLiterals {
+
+ private static final String TABLE_NAME = "tbl";
+
+ /**
+ * Plain ASCII keys, values and comments must keep producing exactly the
same DDL as before: the
+ * escaping must be a no-op when there is nothing to escape.
+ */
+ @Test
+ public void simpleLiteralsAreUnchanged() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+
+ Assertions.assertEquals(
+ "ALTER TABLE tbl SET TBLPROPERTIES ('k1' = 'v1')",
+ handler.changeAndAddProperties(Collections.singletonMap("k1", "v1")));
+ Assertions.assertEquals(
+ "ALTER TABLE tbl UNSET TBLPROPERTIES ('k1')",
+ handler.removeProperties(Collections.singleton("k1")));
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT 'new comment'",
+ handler.changeColumnsComment("c", "new comment"));
+ }
+
+ /** {@code '} must be escaped in both the property key and the property
value. */
+ @Test
+ public void changeAndAddPropertiesEscapesApostropheInKeyAndValue() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ String ddl =
handler.changeAndAddProperties(Collections.singletonMap("user's", "O'Reilly"));
+
+ Assertions.assertEquals("ALTER TABLE tbl SET TBLPROPERTIES ('user\\'s' =
'O\\'Reilly')", ddl);
+ Assertions.assertFalse(ddl.contains("''"), "apostrophe must be
backslash-escaped, not doubled");
+ }
+
+ /** {@code \} must be doubled in both the property key and the property
value. */
+ @Test
+ public void changeAndAddPropertiesEscapesBackslashInKeyAndValue() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ String ddl =
handler.changeAndAddProperties(Collections.singletonMap("a\\b", "C:\\tmp"));
+
+ Assertions.assertEquals("ALTER TABLE tbl SET TBLPROPERTIES ('a\\\\b' =
'C:\\\\tmp')", ddl);
+ }
+
+ /** {@code \} must be doubled in a removed property key as well. */
+ @Test
+ public void removePropertiesEscapesBackslashInKey() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ Assertions.assertEquals(
+ "ALTER TABLE tbl UNSET TBLPROPERTIES ('a\\\\b')",
+ handler.removeProperties(Collections.singleton("a\\b")));
+ }
+
+ /** A column comment containing both an apostrophe and a backslash. */
+ @Test
+ public void changeColumnsCommentEscapesApostropheAndBackslash() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT 'O\\'Reilly\\'s C:\\\\tmp'",
+ handler.changeColumnsComment("c", "O'Reilly's C:\\tmp"));
+ }
+
+ /**
+ * A backslash immediately before an apostrophe must be escaped
backslash-first, leaving the quote
+ * unambiguous instead of a stray unterminated quote.
+ */
+ @Test
+ public void backslashImmediatelyBeforeQuoteIsEscapedInOrder() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT 'a\\\\\\'b'",
+ handler.changeColumnsComment("c", "a\\'b"));
+ }
+
+ /** A raw newline or tab must be emitted escaped so the comment value
survives. */
+ @Test
+ public void changeColumnsCommentEscapesNewlineAndTab() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT 'line1\\nline2\\tend'",
+ handler.changeColumnsComment("c", "line1\nline2\tend"));
+ }
+
+ /** A null comment removes the comment, which Spark spells as an empty
literal. */
+ @Test
+ public void changeColumnsCommentNullBecomesEmptyLiteral() {
+ SparkMetadataChangeHandler handler = new
SparkMetadataChangeHandler(TABLE_NAME);
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT ''",
handler.changeColumnsComment("c", null));
+ }
+
+ /**
+ * A comment removal arriving through the real {@link DDLReverser} history
path must also emit an
+ * empty literal rather than the text {@code null}.
+ */
+ @Test
+ public void ddlReverserRemovingCommentEmitsEmptyLiteral() {
+ List<DDLInfo> ddls =
+ reverse(
+ meta(0L, "old comment", Collections.emptyMap()),
+ meta(1L, null, Collections.emptyMap()));
+
+ Assertions.assertEquals(1, ddls.size());
+ Assertions.assertEquals("ALTER TABLE tbl ALTER COLUMN c COMMENT ''",
ddls.get(0).getDdl());
+ Assertions.assertEquals(DDLInfo.DDLType.UPDATE_SCHEMA,
ddls.get(0).getDdlType());
+ }
+
+ /** Drives the {@link DDLReverser} property history (removed key, changed
value) and comment. */
+ @Test
+ public void ddlReverserEscapesPropertyAndCommentHistory() {
+ Map<String, String> preProperties = new LinkedHashMap<>();
+ preProperties.put("d\\rop", "x");
+ preProperties.put("k", "old");
+
+ List<DDLInfo> ddls =
+ reverse(
+ meta(0L, "old comment", preProperties),
+ meta(1L, "it's", Collections.singletonMap("k", "O'Reilly")));
+
+ Assertions.assertEquals(3, ddls.size());
+
+ Assertions.assertEquals(
+ "ALTER TABLE tbl UNSET TBLPROPERTIES ('d\\\\rop')",
ddls.get(0).getDdl());
+ Assertions.assertEquals(DDLInfo.DDLType.UPDATE_PROPERTIES,
ddls.get(0).getDdlType());
+
+ Assertions.assertEquals(
+ "ALTER TABLE tbl SET TBLPROPERTIES ('k' = 'O\\'Reilly')",
ddls.get(1).getDdl());
+ Assertions.assertEquals(DDLInfo.DDLType.UPDATE_PROPERTIES,
ddls.get(1).getDdlType());
+
+ Assertions.assertEquals(
+ "ALTER TABLE tbl ALTER COLUMN c COMMENT 'it\\'s'",
ddls.get(2).getDdl());
+ Assertions.assertEquals(DDLInfo.DDLType.UPDATE_SCHEMA,
ddls.get(2).getDdlType());
+ }
+
+ /** Drives {@link DDLReverser} with two fake metadata snapshots. */
+ private static List<DDLInfo> reverse(
+ TableMetaExtract.InternalTableMeta pre,
TableMetaExtract.InternalTableMeta current) {
+ TableMetaExtract<String> extractor = table -> Arrays.asList(pre, current);
+ return new DDLReverser<>(extractor)
+ .reverse("table", TableIdentifier.of("catalog", "db", TABLE_NAME));
+ }
+
+ private static TableMetaExtract.InternalTableMeta meta(
+ long time, String comment, Map<String, String> properties) {
+ return new TableMetaExtract.InternalTableMeta(
+ time, Collections.singletonList(schema(0, comment, false)),
properties);
+ }
+
+ private static TableMetaExtract.InternalSchema schema(int id, String
comment, boolean required) {
+ return new TableMetaExtract.InternalSchema(id, null, "c", "int", comment,
required);
+ }
+}
