github-actions[bot] commented on PR #62312: URL: https://github.com/apache/doris/pull/62312#issuecomment-4221391372
No issues found in this review. Critical checkpoints: - Goal and correctness: The PR cleanly extracts descriptor-to-Thrift serialization into `DescriptorToThriftConverter`, updates the FE call sites consistently, and preserves behavior based on the reviewed call chains. - Scope/minimality: The change stays focused on FE descriptor serialization and access-path type cleanup without unrelated logic changes. - Concurrency/lifecycle/config/compatibility: No new concurrency, lifecycle, config, persistence, or rolling-upgrade compatibility concerns were introduced. - Parallel paths: The relevant FE serialization paths and nested-column/variant access-path plumbing were updated consistently. - Conditional checks/observability: No new risky conditional branches or observability gaps stood out. - Testing: Added or updated FE unit tests cover the converter and nested/variant pruning paths. Local Maven validation in this runner was blocked because `fe-core` depends on the unpublished snapshot artifact `org.apache.doris:fe-foundation:1.2-SNAPSHOT`. Residual risk: `META` access-path handling still appears dormant in the current pruning logic, so if that path is enabled later it should get dedicated behavioral tests. -- 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]
