This is an automated email from the ASF dual-hosted git repository.
jinsongzhou 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 697d60484 [AMORO-3682]Fix table creation failure when comment contains
`PRIMARY KEY` text (#3683)
697d60484 is described below
commit 697d6048436a602128c2548077081bcad1dae690
Author: ZhouJinsong <[email protected]>
AuthorDate: Fri Aug 1 11:51:33 2025 +0800
[AMORO-3682]Fix table creation failure when comment contains `PRIMARY KEY`
text (#3683)
Fix creating table error with comment contains primary key
---
.../parser/MixedFormatSqlExtendAstBuilder.scala | 6 +-
.../spark/test/suites/sql/TestCreateTableSQL.java | 97 ++++++++++++++++++++++
.../parser/MixedFormatSqlExtendAstBuilder.scala | 6 +-
.../spark/test/suites/sql/TestCreateTableSQL.java | 97 ++++++++++++++++++++++
4 files changed, 202 insertions(+), 4 deletions(-)
diff --git
a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
index c65d3e7d4..f22c36301 100644
---
a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
+++
b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
@@ -198,8 +198,10 @@ class MixedFormatSqlExtendAstBuilder()
* Create a comment string.
*/
override def visitPrimarySpec(ctx:
MixedFormatSqlExtendParser.PrimarySpecContext): Seq[String] =
- withOrigin(ctx) {
- visitIdentifierList(ctx.identifierList())
+ Option(ctx).fold(Seq.empty[String]) { validCtx =>
+ withOrigin(validCtx) {
+ visitIdentifierList(validCtx.identifierList())
+ }
}
override def visitCreateTableClauses(ctx:
MixedFormatSqlExtendParser.CreateTableClausesContext)
diff --git
a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
index 6142def5a..90b8cb3ad 100644
---
a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
+++
b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
@@ -32,6 +32,7 @@ import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.Types;
+import org.junit.Assert;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
@@ -309,4 +310,100 @@ public class TestCreateTableSQL extends
MixedTableTestBase {
expectSchema, PartitionSpec.unpartitioned(),
hiveTable.getSd().getCols());
}
}
+
+ public static Stream<Arguments> testPrimaryKeyComment() {
+ String structDDL =
+ "id INT comment 'primary key id',\n"
+ + "data string NOT NULL,\n"
+ + "point struct<x: double NOT NULL, y: double NOT NULL>,\n"
+ + "maps map<string, string>,\n"
+ + "arrays array<string>,\n"
+ + "pt string ";
+ Types.NestedField id =
+ Types.NestedField.optional(1, "id", Types.IntegerType.get(), "primary
key id");
+ Types.NestedField data = Types.NestedField.required(2, "data",
Types.StringType.get());
+ Types.NestedField point =
+ Types.NestedField.optional(
+ 3,
+ "point",
+ Types.StructType.of(
+ Types.NestedField.required(4, "x", Types.DoubleType.get()),
+ Types.NestedField.required(5, "y", Types.DoubleType.get())));
+ Types.NestedField map =
+ Types.NestedField.optional(
+ 6,
+ "maps",
+ Types.MapType.ofOptional(7, 8, Types.StringType.get(),
Types.StringType.get()));
+ Types.NestedField array =
+ Types.NestedField.optional(
+ 9, "arrays", Types.ListType.ofOptional(10,
Types.StringType.get()));
+ Types.NestedField pt = Types.NestedField.optional(11, "pt",
Types.StringType.get());
+
+ return Stream.of(
+ Arguments.of(
+ TableFormat.MIXED_ICEBERG,
+ structDDL,
+ "TBLPROPERTIES('key'='value1', 'catalog'='INTERNAL')",
+ new Schema(Lists.newArrayList(id, data, point, map, array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "INTERNAL"),
+ "primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_ICEBERG,
+ structDDL + ", PRIMARY KEY(id)",
+ "TBLPROPERTIES('key'='value1', 'catalog'='INTERNAL')",
+ new Schema(Lists.newArrayList(id.asRequired(), data, point, map,
array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "INTERNAL"),
+ "primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_HIVE,
+ structDDL,
+ "tblproperties('key'='value1', 'catalog'='hive')",
+ new Schema(Lists.newArrayList(id, data, point, map, array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "hive"),
+ "Primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_HIVE,
+ structDDL + ", PRIMARY KEY(id)",
+ "tblproperties('key'='value1', 'catalog'='hive')",
+ new Schema(Lists.newArrayList(id.asRequired(), data, point, map,
array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "hive"),
+ "Primary key comment"));
+ }
+
+ @DisplayName("Test comment contains primary key")
+ @ParameterizedTest
+ @MethodSource
+ public void testPrimaryKeyComment(
+ TableFormat format,
+ String structDDL,
+ String propertiesDDL,
+ Schema expectSchema,
+ Map<String, String> expectProperties,
+ String tableComment) {
+
spark().conf().set(SparkSQLProperties.USE_TIMESTAMP_WITHOUT_TIME_ZONE_IN_NEW_TABLES,
false);
+ String sqlText =
+ "CREATE TABLE "
+ + target()
+ + "("
+ + structDDL
+ + ") using "
+ + provider(format)
+ + " "
+ + propertiesDDL
+ + " comment '"
+ + tableComment
+ + "'";
+ sql(sqlText);
+
+ MixedTable tbl = loadTable();
+
+ Asserts.assertType(expectSchema.asStruct(), tbl.schema().asStruct());
+ Asserts.assertHashMapContainExpect(expectProperties, tbl.properties());
+ Assert.assertEquals(tableComment, tbl.properties().get("comment"));
+ if (TableFormat.MIXED_HIVE.equals(format)) {
+ Table hiveTable = loadHiveTable();
+ Asserts.assertHiveColumns(
+ expectSchema, PartitionSpec.unpartitioned(),
hiveTable.getSd().getCols());
+ }
+ }
}
diff --git
a/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
b/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
index ff6738ce3..a0b79ba2e 100644
---
a/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
+++
b/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/main/scala/org/apache/spark/sql/amoro/parser/MixedFormatSqlExtendAstBuilder.scala
@@ -200,8 +200,10 @@ class MixedFormatSqlExtendAstBuilder()
* Create a comment string.
*/
override def visitPrimarySpec(ctx:
MixedFormatSqlExtendParser.PrimarySpecContext): Seq[String] =
- withOrigin(ctx) {
- visitIdentifierList(ctx.identifierList())
+ Option(ctx).fold(Seq.empty[String]) { validCtx =>
+ withOrigin(validCtx) {
+ visitIdentifierList(validCtx.identifierList())
+ }
}
override def visitCreateTableClauses(ctx:
MixedFormatSqlExtendParser.CreateTableClausesContext)
diff --git
a/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
b/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
index 7d79ebf44..53bbb1c4b 100644
---
a/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
+++
b/amoro-format-mixed/amoro-mixed-spark/v3.5/amoro-mixed-spark-3.5/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java
@@ -32,6 +32,7 @@ import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.types.Type;
import org.apache.iceberg.types.Types;
+import org.junit.Assert;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
@@ -308,4 +309,100 @@ public class TestCreateTableSQL extends
MixedTableTestBase {
expectSchema, PartitionSpec.unpartitioned(),
hiveTable.getSd().getCols());
}
}
+
+ public static Stream<Arguments> testPrimaryKeyComment() {
+ String structDDL =
+ "id INT comment 'primary key id',\n"
+ + "data string NOT NULL,\n"
+ + "point struct<x: double NOT NULL, y: double NOT NULL>,\n"
+ + "maps map<string, string>,\n"
+ + "arrays array<string>,\n"
+ + "pt string ";
+ Types.NestedField id =
+ Types.NestedField.optional(1, "id", Types.IntegerType.get(), "primary
key id");
+ Types.NestedField data = Types.NestedField.required(2, "data",
Types.StringType.get());
+ Types.NestedField point =
+ Types.NestedField.optional(
+ 3,
+ "point",
+ Types.StructType.of(
+ Types.NestedField.required(4, "x", Types.DoubleType.get()),
+ Types.NestedField.required(5, "y", Types.DoubleType.get())));
+ Types.NestedField map =
+ Types.NestedField.optional(
+ 6,
+ "maps",
+ Types.MapType.ofOptional(7, 8, Types.StringType.get(),
Types.StringType.get()));
+ Types.NestedField array =
+ Types.NestedField.optional(
+ 9, "arrays", Types.ListType.ofOptional(10,
Types.StringType.get()));
+ Types.NestedField pt = Types.NestedField.optional(11, "pt",
Types.StringType.get());
+
+ return Stream.of(
+ Arguments.of(
+ TableFormat.MIXED_ICEBERG,
+ structDDL,
+ "TBLPROPERTIES('key'='value1', 'catalog'='INTERNAL')",
+ new Schema(Lists.newArrayList(id, data, point, map, array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "INTERNAL"),
+ "primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_ICEBERG,
+ structDDL + ", PRIMARY KEY(id)",
+ "TBLPROPERTIES('key'='value1', 'catalog'='INTERNAL')",
+ new Schema(Lists.newArrayList(id.asRequired(), data, point, map,
array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "INTERNAL"),
+ "primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_HIVE,
+ structDDL,
+ "tblproperties('key'='value1', 'catalog'='hive')",
+ new Schema(Lists.newArrayList(id, data, point, map, array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "hive"),
+ "Primary key comment"),
+ Arguments.of(
+ TableFormat.MIXED_HIVE,
+ structDDL + ", PRIMARY KEY(id)",
+ "tblproperties('key'='value1', 'catalog'='hive')",
+ new Schema(Lists.newArrayList(id.asRequired(), data, point, map,
array, pt)),
+ ImmutableMap.of("key", "value1", "catalog", "hive"),
+ "Primary key comment"));
+ }
+
+ @DisplayName("Test comment contains primary key")
+ @ParameterizedTest
+ @MethodSource
+ public void testPrimaryKeyComment(
+ TableFormat format,
+ String structDDL,
+ String propertiesDDL,
+ Schema expectSchema,
+ Map<String, String> expectProperties,
+ String tableComment) {
+
spark().conf().set(SparkSQLProperties.USE_TIMESTAMP_WITHOUT_TIME_ZONE_IN_NEW_TABLES,
false);
+ String sqlText =
+ "CREATE TABLE "
+ + target()
+ + "("
+ + structDDL
+ + ") using "
+ + provider(format)
+ + " "
+ + propertiesDDL
+ + " comment '"
+ + tableComment
+ + "'";
+ sql(sqlText);
+
+ MixedTable tbl = loadTable();
+
+ Asserts.assertType(expectSchema.asStruct(), tbl.schema().asStruct());
+ Asserts.assertHashMapContainExpect(expectProperties, tbl.properties());
+ Assert.assertEquals(tableComment, tbl.properties().get("comment"));
+ if (TableFormat.MIXED_HIVE.equals(format)) {
+ Table hiveTable = loadHiveTable();
+ Asserts.assertHiveColumns(
+ expectSchema, PartitionSpec.unpartitioned(),
hiveTable.getSd().getCols());
+ }
+ }
}
