raghav-reglobe commented on PR #65989: URL: https://github.com/apache/doris/pull/65989#issuecomment-5066509038
Thanks @Doris-Breakwater for the careful review — the required-field catch is exactly right. Skipping `setComment` entirely would have left the required Thrift field unset and failed response validation, which is worse than the bug. Updated the PR: - The comment is now always set: `""` when `TABLE_COMMENT` is not projected, the real value when it is (or when `required_columns` is unset — older BE, unchanged behavior). No Thrift contract change. - Added the three `FrontendServiceImplTest` cases you suggested: no-`TABLE_COMMENT` projection (verifies `getComment()` is never called, empty placeholder, and `status.validate()` passes), positive `TABLE_COMMENT` projection, and unset `required_columns`. - Extended `test_tables_status_column_pruning.groovy` with a projection excluding `TABLE_COMMENT` (asserts `col=TABLE_COMMENT` is not requested) and a positive one including it. On the missing info: the reproduction was on a 2026-07-22 master build (merge base `ab043a433c` era). I'll post before/after wall time + per-table loadTable/GET counts from the same REST catalogs once our next build including this patch has run. -- 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]
