yuqi1129 commented on code in PR #12007:
URL: https://github.com/apache/gravitino/pull/12007#discussion_r3614097471
##########
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:
Good question. The new regex stays compatible with existing data:
1. **Marker part** — `(From Gravitino, DO NOT EDIT: gravitino.v1.uid...)`
matches exactly the format `StringIdentifier` writes, which is the only marker
format that has ever been persisted (`CURRENT_FORMAT_VERSION = 1`). So every
comment Gravitino has written is still matched.
2. **Comment body** — the new group `(?:\\.|""|[^"\\])*` is actually *more*
tolerant than the old `[^\(]+?`: the old pattern failed whenever the comment
itself contained a `(`, while the new one handles that fine.
3. The only stricter requirement is that quotes must be escaped — but a
comment containing a raw `"` already produced malformed SQL under the previous
write path, so there is no valid stored data that relied on the old, unescaped
form.
So for anything Gravitino previously wrote, the new regex parses it the same
or better. The round-trip stays covered by the escaping/unescaping tests.
--
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]
