wgtmac commented on PR #701: URL: https://github.com/apache/iceberg-cpp/pull/701#issuecomment-5141558339
Thanks for the work on this PR! I did a detailed code review and pushed the changes directly to the branch to avoid a long review-and-revision loop. Summary of the changes: - Aligned table full-name handling with Java’s catalog-owned design and preserved REST’s catalog-specific naming behavior. - Removed unnecessary public APIs, including reporter-combination and legacy table-scan builder APIs. - Simplified commit reporting so each snapshot update owns its metrics and reports once, only after a successful standalone or transaction commit. - Reused retry-runner attempt accounting and removed redundant commit-attempt state. - Aligned scan metrics accounting and naming across table scans, manifest planning, delete-file indexing, and deletion vectors. - Improved Java parity for expression sanitization, including temporal values, hashes, transforms, unary predicates, and overflow handling. - Reworked catalog reporter initialization and executor handling. REST reporting now supports both asynchronous execution with a user-provided executor and synchronous fallback without one. - Propagated catalog reporter construction errors through `Result` instead of silently ignoring them. - Strengthened and simplified the related tests, fixtures, names, and comments. -- 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]
