dataroaring commented on PR #60583: URL: https://github.com/apache/doris/pull/60583#issuecomment-3864858587
### Code review Found 2 issues: 1. **Inverted `is_disposable` logic in `beta_rowset_reader.cpp`** -- Missing `!` negation. The old code was `is_disposable = disable_file_cache`, so when cache is disabled, data is disposable (correct). The new code is `is_disposable = enable_file_cache_olap_tables`, which marks data as disposable when cache is *enabled* (wrong). Should be `!enable_file_cache_olap_tables`, consistent with how `file_scanner.cpp` handles the same pattern. https://github.com/apache/doris/blob/793f19249baeba128d824e88b50e62c35ea9745e/be/src/olap/rowset/beta_rowset_reader.cpp#L222-L225 2. **Wrong variable used in `file_scanner.cpp`** -- `FileScanner` is used for external catalog scans (Hive, S3, etc.), but line 172 uses `enable_file_cache_olap_tables` instead of `enable_file_cache_external_catalogs`. Line 1815 in the same file correctly uses `enable_file_cache_external_catalogs`, confirming the variable at line 172 should match. This defeats the purpose of separating OLAP vs external catalog cache control. https://github.com/apache/doris/blob/793f19249baeba128d824e88b50e62c35ea9745e/be/src/vec/exec/scan/file_scanner.cpp#L171-L173 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- 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]
