diqiu50 commented on code in PR #13975:
URL: https://github.com/apache/gravitino/pull/13975#discussion_r4325070832


##########
catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java:
##########
@@ -1235,6 +1259,11 @@ private StringBuilder appendColumnDefinitionForModify(
   }
 
   private StringBuilder appendColumnDefinition(JdbcColumn column, 
StringBuilder sqlBuilder) {
+    return appendColumnDefinition(column, sqlBuilder, false);

Review Comment:
   updateColumnNullabilityDefinition and updateColumnPositionFieldDefinition 
still use appendColumnDefinition(column, sb), which always passes 
useDoubleQuotedStringDefault=false. On Doris 2.1+, changing the nullability or 
position of a column with a default like O'Brien would re-render it as DEFAULT 
'O''Brien'. Should these two paths also use the version-aware rendering, like 
ADD/MODIFY COLUMN?



##########
catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java:
##########


Review Comment:
   Now that property values can contain escaped quotes, the greedy key group in 
"(.*)"\s*=\s*"(.*)",? can split them at the wrong place. For example, k = a" = 
"b is generated as "k" = "a"" = ""b", and it is parsed back as key k" = "a" 
with value "b. Maybe use a pattern that understands doubled quotes, e.g. 
"((?:[^"]|"")*)"\s*=\s*"((?:[^"]|"")*)"?



-- 
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]

Reply via email to