github-actions[bot] commented on PR #61939: URL: https://github.com/apache/doris/pull/61939#issuecomment-4165852249
No issues found in this review. Critical checkpoints: - Goal/test fit: The PR goal is to preserve JDBC `query()` TVF column aliases, and the code change accomplishes that on the `ResultSetMetaData` path used by `getColumnsFromQuery()`. Coverage includes a focused FE unit test and a regression test for both `desc function query(...)` and querying aliased columns. - Change scope: The modification is small and focused to `JdbcFieldSchema(ResultSetMetaData, int)`, which is the constructor used only for query-derived schemas, so regular JDBC table schema discovery remains unchanged. - Concurrency/lifecycle: No new concurrency, locking, lifecycle, config, compatibility, persistence, or FE/BE protocol concerns were introduced in this diff. - Parallel paths: I checked the adjacent JDBC schema-discovery path in `JdbcClient`; only the query-metadata path is changed, which matches the stated bug. - Conditional logic: The added fallback from `getColumnLabel()` to `getColumnName()` is straightforward and appropriate for drivers that return an empty label. - Test coverage/observability: Tests are present for the user-visible behavior. No additional observability looks necessary for this small metadata fix. - Performance: No meaningful performance impact; the change just swaps one metadata accessor for another with a safe fallback. - Other issues: None found from the reviewed code paths. -- 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]
