Copilot commented on code in PR #65325:
URL: https://github.com/apache/doris/pull/65325#discussion_r3535828666


##########
fs_brokers/cdc_client/src/main/java/org/apache/doris/cdcclient/utils/SchemaChangeHelper.java:
##########
@@ -136,6 +106,97 @@ public static String columnToDorisType(Column column) {
         return pgTypeNameToDorisType(column.typeName(), column.length(), 
column.scale().orElse(-1));
     }
 
+    /** Convert a Debezium MySQL Column to a Doris column type string. */
+    public static String mysqlColumnToDorisType(Column column) {
+        Preconditions.checkNotNull(column.typeName());
+        String mysqlTypeName = column.typeName().toUpperCase();
+        String[] typeFields = mysqlTypeName.split(" ");
+        String mysqlType = typeFields[0];
+        boolean unsigned = typeFields.length > 1 && 
"UNSIGNED".equals(typeFields[1]);
+        int length = column.length();

Review Comment:
   mysqlColumnToDorisType() only treats the type as unsigned when "UNSIGNED" is 
the 2nd token (typeFields[1]). This misses valid MySQL forms like `INT 
ZEROFILL` (ZEROFILL implies UNSIGNED) and is also brittle if extra qualifiers 
appear in different positions. This can map unsigned integer ranges to signed 
Doris types.



##########
fs_brokers/cdc_client/src/test/java/org/apache/doris/cdcclient/source/parse/mysql/CustomMySqlAntlrDdlParserTest.java:
##########
@@ -0,0 +1,134 @@
+// 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.doris.cdcclient.source.parse.mysql;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.debezium.relational.Column;
+import io.debezium.relational.Table;
+import io.debezium.relational.TableEditor;
+import io.debezium.relational.TableId;
+import io.debezium.relational.Tables;
+
+import java.sql.Types;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+class CustomMySqlAntlrDdlParserTest {
+    private static final TableId TABLE = new TableId("db1", null, "t1");
+
+    @Test
+    void parseAddColumn() {
+        Tables tables = tables(table("id"));
+        CustomMySqlAntlrDdlParser parser = parser();
+
+        parser.parse("ALTER TABLE t1 ADD COLUMN age INT", tables);
+
+        List<MySqlSchemaChange> changes = parser.getAndClearParsedChanges();
+        assertEquals(1, changes.size());
+        assertEquals(MySqlSchemaChange.Type.ADD, changes.get(0).getType());
+        assertEquals(TABLE, changes.get(0).getTableId());
+        assertEquals("age", changes.get(0).getColumn().name());
+        assertTrue(parser.getAndClearParsedChanges().isEmpty());
+    }
+
+    @Test
+    void parseAddColumnComment() {
+        Tables tables = tables(table("id"));
+        CustomMySqlAntlrDdlParser parser = parser();
+
+        parser.parse("ALTER TABLE t1 ADD COLUMN city VARCHAR(30) COMMENT 'user 
city'", tables);
+
+        List<MySqlSchemaChange> changes = parser.getAndClearParsedChanges();
+        assertEquals(1, changes.size());
+        assertEquals("city", changes.get(0).getColumn().name());
+        assertEquals("user city", changes.get(0).getColumn().comment());
+    }

Review Comment:
   This test only asserts the parsed column name/comment, but not that the 
parser populated the column's type/length metadata that the production path 
relies on (SchemaChangeHelper.mysqlColumnToDorisType uses 
Column.typeName()/length()/scale()). Adding a couple of assertions here would 
catch regressions where the DDL parser fails to preserve VARCHAR length, etc.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to