diqiu50 commented on PR #13588: URL: https://github.com/apache/gravitino/pull/13588#issuecomment-5886769841
Thanks for the fix. A few comments: 1. If the comment lookup or `ALTER TABLE ... MODIFY COMMENT` fails after `CREATE TABLE` succeeds, the table stays in Doris without the Gravitino ID, and a retry hits `TableAlreadyExistsException`. Consider a best-effort `DROP TABLE` on failure, or at least a clear error message and doc note that the table must be dropped before retrying. 2. Please add a unit test for escaping in the repair path (e.g. a comment containing `"` and `\`). The current unit tests would still pass without `escapeSqlLiteral`. 3. Minor: `loadTableComment` returns `""` for a missing row, which is indistinguishable from an empty comment. Consider throwing `NoSuchTableException` or returning `Optional`. -- 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]
