[fix](struct) Keep nested field comments and names

be/src/information_schema/schema_columns_scanner.cpp

@@ -174,6 +174,29 @@ std::string SchemaColumnsScanner::_to_mysql_data_type_string(TColumnDesc& desc)
     }
 }
 
+namespace {
+
+// A struct field name goes into COLUMN_TYPE next to the separators that give the text its
+// shape. A name holding one of them would make the text impossible to split back into
+// fields, so wrap such a name in back quotes and double any back quote inside it.
+std::string quote_struct_field_name(const std::string& name) {
+    if (name.find_first_of(",:<>`") == std::string::npos) {
+        return name;
+    }
+    std::string quoted = "`";
+    for (char ch : name) {
+        if (ch == '`') {
+            quoted += "``";
+        } else {
+            quoted += ch;
+        }
+    }
+    quoted += "`";
+    return quoted;
+}
+
+} // namespace
+
 std::string SchemaColumnsScanner::_type_to_string(TColumnDesc& desc) {
     switch (desc.columnType) {
     case TPrimitiveType::BOOLEAN:
@@ -284,11 +307,13 @@ std::string SchemaColumnsScanner::_type_to_string(TColumnDesc& desc) {
     case TPrimitiveType::STRUCT: {
         // for old be service we should compitable
         std::string ret = "struct<";
-        if (!desc.children.empty()) {
-            for (int i = 0; i < desc.children.size() - 1; ++i) {
-                ret += _type_to_string(desc.children[i]) + ",";
+        // Name every field, a client rebuilds the schema from this text.
+        for (size_t i = 0; i < desc.children.size(); ++i) {
+            if (i != 0) {
+                ret += ",";
             }
-            ret += _type_to_string(desc.children[desc.children.size() - 1]);
+            ret += quote_struct_field_name(desc.children[i].columnName) + ":" +
+                   _type_to_string(desc.children[i]);
         }
         ret += ">";
         return ret;

be/test/exec/schema_scanner/schema_columns_scanner_test.cpp

@@ -0,0 +1,111 @@
+// 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.
+
+#include "information_schema/schema_columns_scanner.h"
+
+#include <gen_cpp/FrontendService_types.h>
+#include <gen_cpp/Types_types.h>
+#include <gtest/gtest.h>
+
+#include <string>
+
+namespace doris {
+
+namespace {
+
+TColumnDesc make_desc(const std::string& name, TPrimitiveType::type type) {
+    TColumnDesc desc;
+    desc.__set_columnName(name);
+    desc.__set_columnType(type);
+    return desc;
+}
+
+} // namespace
+
+// COLUMN_TYPE is the text information_schema clients read a schema back from, so a
+// STRUCT has to name its fields there. The names arrive lower cased from the FE, while
+// SHOW CREATE TABLE keeps the spelling the user wrote.
+class SchemaColumnsScannerTest : public testing::Test {
+protected:
+    std::string type_to_string(TColumnDesc& desc) { return _scanner._type_to_string(desc); }
+
+private:
+    SchemaColumnsScanner _scanner;
+};
+
+TEST_F(SchemaColumnsScannerTest, struct_type_string_carries_field_names) {
+    TColumnDesc desc = make_desc("st", TPrimitiveType::STRUCT);
+    desc.__set_children(
+            {make_desc("f1", TPrimitiveType::INT), make_desc("f2", TPrimitiveType::STRING)});
+
+    EXPECT_EQ("struct<f1:int(11),f2:string>", type_to_string(desc));
+}
+
+TEST_F(SchemaColumnsScannerTest, nested_struct_type_string_carries_field_names) {
+    TColumnDesc element = make_desc("item", TPrimitiveType::STRUCT);
+    element.__set_children({make_desc("deep", TPrimitiveType::INT)});
+
+    // An array element has no name of its own, only the struct fields are named.
+    TColumnDesc arr = make_desc("arr", TPrimitiveType::ARRAY);
+    arr.__set_children({element});
+
+    TColumnDesc outer = make_desc("outer", TPrimitiveType::STRUCT);
+    outer.__set_children({arr});
+
+    EXPECT_EQ("struct<arr:array<struct<deep:int(11)>>>", type_to_string(outer));
+}
+
+TEST_F(SchemaColumnsScannerTest, struct_field_name_holding_a_separator_is_quoted) {
+    // These names are legal, they are declared with back quotes. Printed raw they would make
+    // the text impossible to split back into fields.
+    TColumnDesc desc = make_desc("st", TPrimitiveType::STRUCT);
+    desc.__set_children(
+            {make_desc("a,b", TPrimitiveType::INT), make_desc("c:d", TPrimitiveType::INT),
+             make_desc("e<f>", TPrimitiveType::INT), make_desc("g`h", TPrimitiveType::INT)});
+
+    EXPECT_EQ("struct<`a,b`:int(11),`c:d`:int(11),`e<f>`:int(11),`g``h`:int(11)>",
+              type_to_string(desc));
+}
+
+TEST_F(SchemaColumnsScannerTest, plain_struct_field_name_stays_bare) {
+    TColumnDesc desc = make_desc("st", TPrimitiveType::STRUCT);
+    desc.__set_children({make_desc("a-b", TPrimitiveType::INT),
+                         make_desc("f 2", TPrimitiveType::INT),
+                         make_desc("order", TPrimitiveType::INT)});
+
+    // A hyphen, a space or a keyword carries no meaning in this text, so leave them alone.
+    EXPECT_EQ("struct<a-b:int(11),f 2:int(11),order:int(11)>", type_to_string(desc));
+}
+
+TEST_F(SchemaColumnsScannerTest, empty_struct_type_string) {
+    TColumnDesc desc = make_desc("st", TPrimitiveType::STRUCT);
+    EXPECT_EQ("struct<>", type_to_string(desc));
+}
+
+// Array and map keep printing types only, they have no field names to carry.
+TEST_F(SchemaColumnsScannerTest, array_and_map_type_string_unchanged) {
+    TColumnDesc arr = make_desc("arr", TPrimitiveType::ARRAY);
+    arr.__set_children({make_desc("item", TPrimitiveType::INT)});
+    EXPECT_EQ("array<int(11)>", type_to_string(arr));
+
+    TColumnDesc mp = make_desc("mp", TPrimitiveType::MAP);
+    mp.__set_children(
+            {make_desc("key", TPrimitiveType::STRING), make_desc("value", TPrimitiveType::INT)});
+    EXPECT_EQ("map<string,int(11)>", type_to_string(mp));
+}
+
+} // namespace doris

fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java

@@ -884,18 +884,25 @@ public String toSql(boolean isUniqueTable) {
     }
 
     public String toSql(boolean isUniqueTable, boolean isCompatible) {
-        return toSql(isUniqueTable, isCompatible, false);
+        return toSql(isUniqueTable, isCompatible, false, false);
     }
 
-    /** Use a placeholder only for internal CREATE TABLE LIKE parsing; restore the expression before analysis. */
-    public String toSql(boolean isUniqueTable, boolean isCompatible, boolean useGeneratedColumnPlaceholder) {
+    /**
+     * Use a placeholder only for internal CREATE TABLE LIKE parsing; restore the expression before analysis.
+     * Pass the SQL mode the statement will be parsed under, a comment is escaped differently under
+     * NO_BACKSLASH_ESCAPES.
+     */
+    public String toSql(boolean isUniqueTable, boolean isCompatible, boolean useGeneratedColumnPlaceholder,
+            boolean noBackslashEscapes) {
         StringBuilder sb = new StringBuilder();
         sb.append("`").append(name).append("` ");
         String typeStr = type.toSql();
 
         // show change datetimeV2/dateV2 to datetime/date
         if (isCompatible) {
-            sb.append(type.hideVersionForVersionColumn(true));
+            // isToSql = true, showNestedComment = true
+            // SHOW CREATE TABLE and CREATE TABLE LIKE need the nested comment.
+            sb.append(type.hideVersionForVersionColumn(true, true, noBackslashEscapes));
         } else {
             sb.append(typeStr);
         }

fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java

@@ -4330,6 +4330,13 @@ public static void getCreateTableLikeStmt(CreateTableLikeInfo createTableLikeInf
         sb.append(" (\n");
         int idx = 0;
         List<Column> columns = table.getBaseSchema(false);
+        // The emitted statement is parsed again by CREATE TABLE LIKE and by anyone replaying
+        // SHOW CREATE TABLE, so escape comments for the mode that parse will run under. Read it
+        // once here: a synced statement is built on the journal replay thread, which has no
+        // session, and reading the mode per column would clone the whole SessionVariable each time.
+        // Synced DDL travels to another cluster, so pin it to the default mode instead, otherwise
+        // the master and a replaying follower would ship different text for the same table.
+        boolean noBackslashEscapes = !getDdlForSync && SqlModeHelper.hasNoBackSlashEscapes();
         for (Column column : columns) {
             if (idx++ != 0) {
                 sb.append(",\n");
@@ -4338,7 +4345,8 @@ public static void getCreateTableLikeStmt(CreateTableLikeInfo createTableLikeInf
             // sqlalchemy requires this to parse SHOW CREATE TABLE stmt.
             if (table.isManagedTable()) {
                 sb.append("  ").append(
-                        column.toSql(((OlapTable) table).getKeysType() == KeysType.UNIQUE_KEYS, true, true));
+                        column.toSql(((OlapTable) table).getKeysType() == KeysType.UNIQUE_KEYS, true, true,
+                                noBackslashEscapes));
             } else {
                 sb.append("  ").append(column.toSql());
             }
@@ -4684,6 +4692,13 @@ public static void getDdlStmt(Command command, String dbName, TableIf table, Lis
         sb.append(" (\n");
         int idx = 0;
         List<Column> columns = table.getBaseSchema(false);
+        // The emitted statement is parsed again by CREATE TABLE LIKE and by anyone replaying
+        // SHOW CREATE TABLE, so escape comments for the mode that parse will run under. Read it
+        // once here: a synced statement is built on the journal replay thread, which has no
+        // session, and reading the mode per column would clone the whole SessionVariable each time.
+        // Synced DDL travels to another cluster, so pin it to the default mode instead, otherwise
+        // the master and a replaying follower would ship different text for the same table.
+        boolean noBackslashEscapes = !getDdlForSync && SqlModeHelper.hasNoBackSlashEscapes();
         for (Column column : columns) {
             if (idx++ != 0) {
                 sb.append(",\n");
@@ -4692,7 +4707,8 @@ public static void getDdlStmt(Command command, String dbName, TableIf table, Lis
             // sqlalchemy requires this to parse SHOW CREATE TABLE stmt.
             if (table.isManagedTable()) {
                 sb.append("  ").append(
-                        column.toSql(((OlapTable) table).getKeysType() == KeysType.UNIQUE_KEYS, true));
+                        column.toSql(((OlapTable) table).getKeysType() == KeysType.UNIQUE_KEYS, true, false,
+                                noBackslashEscapes));
             } else {
                 sb.append("  ").append(column.toSql());
             }

fe/fe-core/src/test/java/org/apache/doris/catalog/TypeTest.java

@@ -282,4 +282,56 @@ public void testDatetimeV2ScaleMatching() {
         Assertions.assertFalse(dtv2s6.matchesType(timestampNs));
         Assertions.assertFalse(Type.matchExactType(timestampNs, wildcardDatetimeV2, false));
     }
+
+    // ===================== STRUCT nested comment in DDL =====================
+
+    /** SHOW CREATE TABLE has to print the comment of every nested field. */
+    @Test
+    public void testStructDdlKeepsNestedFieldComment() {
+        StructType structType = new StructType(
+                new StructField("a", Type.INT, null, true),
+                new StructField("b", Type.STRING, "field doc", true));
+        Column column = new Column("s", structType, false, null, true, null, "top-level");
+
+        Assertions.assertEquals(
+                "`s` struct<a:int,b:text comment \"field doc\"> NULL COMMENT \"top-level\"",
+                column.toSql(false, true));
+    }
+
+    /** A nested comment has to stay a readable literal, quotes and back slashes included. */
+    @Test
+    public void testStructDdlEscapesNestedFieldComment() {
+        StructType structType = new StructType(
+                new StructField("b", Type.STRING, "owner''s \\path and \"quotes\"", true));
+        Column column = new Column("s", structType, false, null, true, null, null);
+
+        Assertions.assertEquals(
+                "`s` struct<b:text comment \"owner''s \\\\path and \"\"quotes\"\"\"> NULL",
+                column.toSql(false, true));
+    }
+
+    /** The comment has to be escaped for the SQL mode the statement will be parsed under. */
+    @Test
+    public void testStructDdlFollowsNoBackslashEscapesMode() {
+        StructType structType = new StructType(
+                new StructField("b", Type.STRING, "C:\\tmp", true));
+        Column column = new Column("s", structType, false, null, true, null, null);
+
+        // Default mode reads a doubled back slash as one, NO_BACKSLASH_ESCAPES reads it as two.
+        Assertions.assertEquals("`s` struct<b:text comment \"C:\\\\tmp\"> NULL",
+                column.toSql(false, true, false, false));
+        Assertions.assertEquals("`s` struct<b:text comment \"C:\\tmp\"> NULL",
+                column.toSql(false, true, false, true));
+    }
+
+    /** Nested comments belong to DDL, but DESCRIBE only prints them when asked for. */
+    @Test
+    public void testStructDescribeHidesNestedCommentUnlessRequested() {
+        StructType structType = new StructType(
+                new StructField("b", Type.STRING, "field doc", true));
+        Assertions.assertEquals("struct<b:text>",
+                structType.hideVersionForVersionColumn(true, false, false));
+        Assertions.assertEquals("struct<b:text comment \"field doc\">",
+                structType.hideVersionForVersionColumn(true, true, false));
+    }
 }

regression-test/data/datatype_p0/nested_types/ddl/test_struct_nested_comment_ddl.out

@@ -0,0 +1,14 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !show_create --
+struct_nested_comment	CREATE TABLE `struct_nested_comment` (\n  `id` int NULL,\n  `s` struct<a:int,b:text comment "owner''s \\\\path",c:int> NULL COMMENT "top-level",\n  `n` struct<lvl1:struct<lvl2:int comment "deep doc">> NULL,\n  `arr` array<struct<inside:int comment "in array">> NULL\n) ENGINE=OLAP\nDUPLICATE KEY(`id`)\nDISTRIBUTED BY HASH(`id`) BUCKETS 1\nPROPERTIES (\n"replication_allocation" = "tag.location.default: 1",\n"min_load_replica_num" = "-1",\n"is_being_synced" = "false",\n"storage_medium" = "hdd",\n"storage_format" = "V2",\n"inverted_index_storage_format" = "V3",\n"light_schema_change" = "true",\n"disable_auto_compaction" = "false",\n"group_commit_interval_ms" = "10000",\n"group_commit_data_bytes" = "134217728"\n);
+
+-- !column_type --
+arr	array	array<struct<inside:int(11)>>
+id	int	int(11)
+n	struct	struct<lvl1:struct<lvl2:int(11)>>
+s	struct	struct<a:int(11),b:string,c:int(11)>
+
+-- !column_type_quoting --
+id	int(11)
+s	struct<`a,b`:int(11),`c:d`:int(11),`e<f>`:int(11),plain:int(11)>
+