Doris-Breakwater commented on issue #66233: URL: https://github.com/apache/doris/issues/66233#issuecomment-5115833329
Breakwater-GitHub-Analysis-Slot: slot_739ea86fc1ae ## Initial assessment **Confirmed correctness bug on the reported `2215dc476a1f9a1f6edba8c7a056e450165f7001` baseline.** The final remote-to-local mapping is not injective in all modes, although lookup, object-cache keys, and generated IDs require it to be. The trigger is narrow, but once present Doris can address or cache the wrong remote object, so this should be treated as an FE external-catalog correctness issue rather than an enumeration-only defect. The issue currently has no labels or assignee. Maintainers should add the repository's applicable bug and external-catalog/FE labels. ## Code findings - Database and table snapshot loaders only run their generic collision checks under selected lowercase/case-insensitive settings. An exact `RemoteA -> LocalX`, `RemoteB -> LocalX` collision therefore passes in mode 0. - A concrete standard-mapping trigger is a collision between an explicit mapping and an unmapped pass-through name: a remote list containing `RemoteA` and `LocalX`, with only `RemoteA -> LocalX` configured. The JDBC mapper validates explicit entries against one another but cannot detect the pass-through collision without the complete remote-name snapshot. - There is an additional built-in database trigger: `lower_case_database_names=1` lowercases the final local name at `ExternalCatalog.java:583-585`, while the conflict guard at `ExternalCatalog.java:593-594` covers `lower_case_meta_names` and database mode 2, not database mode 1. Thus remote `Foo` and `foo` can both become local `foo` even without a custom hook. - The legacy cache behavior is deterministic from the code: `listNames()` retains duplicate local values, `getRemoteName()` selects the first pair, the object cache overwrites by the shared local key, and `updateCache()` appends pairs. After conflicting incremental events, the reverse-name entry can therefore identify one remote object while the hot object entry contains the other. Replaying the same event also grows the names list. - Cache-bypass table lookup independently scans the freshly loaded pairs with `findFirst()`, so it does not restore addressability and can disagree with a hot object-cache entry depending on list/event order. ## Required fix properties The suggested direction is correct: validate exact local-name uniqueness **after** all mapping and mode-specific transformations, for database scope and table-within-database scope. Keep that exact check separate from the existing mode-aware case-insensitive check so mode 0 continues to allow distinct `Foo` and `foo`. For #65126, constructor/snapshot validation alone is not sufficient for incremental updates if `NameCacheValue.withName()` first removes the existing local name and then adds the new pair. That sequence hides `RemoteA -> LocalX` followed by `RemoteB -> LocalX` from constructor validation. `withName()` should instead: 1. accept an exact replay of the same remote/local pair without adding another entry; 2. reject a different remote name for an occupied exact local name; 3. reject one remote name being rebound to a different local name unless the rename path has explicitly removed the old identity first. The conflict error should include the shared local name and both remote names. Validation must occur before publishing names, object, or ID state. ## Tests needed before closing - Database and table snapshot loads with a mode-0 custom naming hook producing two different remote names for one exact local name. - Explicit-mapping-versus-pass-through collisions, not only two duplicate explicit mappings. - The direct database mode-1 `Foo`/`foo` collision. - Mode-0 `Foo` and `foo` remaining valid when their exact local names are distinct. - Existing lower-case/case-insensitive conflict behavior remaining intact. - Incremental exact replay remaining idempotent, while a different remote name targeting the same local name fails without partially changing names, object-cache, or ID state. - Error assertions covering the shared local name and both remote names. No FE log, query profile, or runtime reproduction is required to confirm the root cause because the failure follows directly from the supplied revision. A focused unit reproduction as above is the only missing evidence needed with the fix. -- 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]
