Gabriel39 commented on PR #68707: URL: https://github.com/apache/doris/pull/68707#issuecomment-5981112659
Reviewed commit `a7b99799c5235919fece14d978bae065b5a945bd`. **Review conclusion: request changes — one managed-branch compatibility regression.** The new [`isLanceAlphanumeric` check](https://github.com/apache/doris/blob/a7b99799c5235919fece14d978bae065b5a945bd/fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceCatalogClient.java#L526-L532) uses JDK 17 character classifications, which use Unicode 13. The pinned Lance 12.0.0 validates names with Rust `is_alphanumeric`; its minimum Rust version is 1.91, whose tables use Unicode 16. These predicates therefore accept different sets of characters. For example, `dev` contains U+1C89 (CYRILLIC CAPITAL LETTER TJE), introduced in Unicode 16. A Lance-created managed branch with this name and a valid namespace-recorded version is accepted by Lance but rejected by the new Java guard with `Invalid Lance branch name`. This also breaks the existing ordinary managed-table query path, not just the new search TVFs: ```sql SELECT * FROM catalog.db.items@branch('name'='dev'); ``` At the merge base, `readManagedBranch` opened the recorded version without this Java character filter. At this head, validation rejects the name before namespace lookup. This is a narrow functional compatibility regression (Major under the review rubric); no data corruption or resource-exhaustion claim is intended. **Suggested fix:** retain the traversal and unsafe-URI checks, but align character acceptance with the pinned Lance contract, through native validation or matching versioned Unicode data. Add coverage for an existing managed branch containing a post-Unicode-13 letter, for both ordinary scans and search TVFs. References: [JDK 17 Character](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Character.html), [Lance v12 branch validation](https://github.com/lance-format/lance/blob/v12.0.0/rust/lance/src/dataset/refs.rs#L990-L1048), [Lance v12 Rust requirement](https://github.com/lance-format/lance/blob/v12.0.0/Cargo.toml#L56), [Rust 1.91 Unicode tables](https://github.com/rust-lang/rust/blob/1.91.0/library/core/src/unicode/unicode_data.rs#L135). Validation scope: two static review rounds covering all 109 changed files and relevant dependency contracts. No builds, tests, or JNI reproduction were run. No additional substantiated blocking findings were identified. -- 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]
