wenzhenghu commented on PR #60583: URL: https://github.com/apache/doris/pull/60583#issuecomment-3868698740
> ### 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) > > - If this code review was useful, please react with 👍. Otherwise, react with 👎. This feature is still under development and self-testing. Since the PR title can no longer be modified on GitHub, I wasn't able to add the draft label. I can request your review at the latest after the Spring Festival holiday. -- 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]
