lxy-9602 commented on PR #462: URL: https://github.com/apache/paimon-cpp/pull/462#issuecomment-6566882246
@realAaronWu Sorry for the late response, and thanks for working on this and for the detailed implementation and documentation. I’m concerned about the maintenance cost of introducing a separate planning and metadata decoding path. Future changes to the manifest format, file metadata, or regular planning semantics would need to be reflected in both implementations, creating a risk of divergence. I would prefer reusing the existing planning path and passing a budget-aware `MemoryPool` through `ScanContextBuilder::WithMemoryPool()`. The custom pool could throw `std::bad_alloc` when an allocation would exceed its budget, and the caller could catch and handle the exception outside the planning call. Of course, paimon-cpp primarily uses `Status` and `Result` for error handling, so existing paths may need adjustments to propagate allocation exceptions safely and clean up resources correctly. Such targeted fixes would be welcome. Some internal container allocations would remain outside this accounting. If those allocations are relatively small and temporary objects are released promptly, I think that is a reasonable tradeoff, provided the API clearly documents that the budget covers pool-managed allocations rather than all planning memory. Accounting for every allocation—especially through assumptions about STL implementation details—seems to introduce substantial maintenance risk. -- 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]
