diqiu50 commented on code in PR #12007:
URL: https://github.com/apache/gravitino/pull/12007#discussion_r3613260337
##########
catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/utils/DorisUtils.java:
##########
Review Comment:
Doris's property-parsing regex "(.*)"\s*=\s*"(.*)" (in DorisUtils.java, line
~86) is greedy and escape-unaware. The PR now lets values legally contain " and
= (via escaping), but only added unescapeSqlLiteral after the match — the regex
itself was never updated. If a value contains both " and =, the regex can split
at the wrong point, silently storing a corrupted key/value pair with no error.
##########
catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/utils/StarRocksUtils.java:
##########
@@ -67,7 +68,9 @@ public class StarRocksUtils {
"(?:^|\\s|\\))DISTRIBUTED\\s+BY\\s+(?:RANDOM\\b|\\w+\\s*\\()",
Pattern.CASE_INSENSITIVE);
private static final Pattern TABLE_COMMENT_PATTERN =
- Pattern.compile("COMMENT\\s*\"([^\\(]+?)\\s*\\(From Gravitino,.*\\)\"");
+ Pattern.compile(
+ "COMMENT\\s*\"((?:\\\\.|\"\"|[^\"\\\\])*)\\s+"
+ + "\\(From Gravitino, DO NOT EDIT:
gravitino\\.v\\d+\\.uid-?\\d+\\)\"");
Review Comment:
The regex is now stricter. How can we remain compatible with existing data?
##########
catalogs/catalog-jdbc-starrocks/src/main/java/org/apache/gravitino/catalog/starrocks/operations/StarRocksTableOperations.java:
##########
@@ -170,7 +171,17 @@ protected String generateAlterTableSql(
} else if (change instanceof TableChange.UpdateComment) {
TableChange.UpdateComment updateComment = (TableChange.UpdateComment)
change;
String newComment = updateComment.getNewComment();
- alterSql.add("MODIFY COMMENT \"" + newComment + "\"");
+ if (StringIdentifier.fromComment(newComment) == null) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ StringIdentifier identifier =
StringIdentifier.fromComment(lazyLoadTable.comment());
+ if (identifier != null) {
+ newComment = StringIdentifier.addToComment(identifier, newComment);
+ }
+ }
+ if (StringUtils.isNotEmpty(newComment)) {
Review Comment:
We need to check the mark is exist
--
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]
