[realAaronWu]

Purpose

Drain accepted manifest tasks before ordinary planning leaves their submission scope after a failure, protecting borrowed scan/snapshot state while preserving the original error.

Reuse ordinary planning and ScanContextBuilder::WithMemoryPool() instead of a separate decoder or STL-specific accounting.

Review follow-up

Added braces to all three guards; shortened the reading guide to an example and key notes; renamed the test file/suite to scan exception safety; adopted the project status assertion macros. Keep the executor submission precondition in the executor API documentation.

Validation

Validation candidate 9baf8542: affected-file formatting, C++/CMake/Sphinx lint and whitespace checks passed. New native runtime/CI results remain pending; the local test host is currently unreachable.

At previous head 9d6ee739, twelve Build and Test jobs and the other three workflows passed. The remaining ARM64 GCC debug job failed while downloading dependencies, before running tests.

Earlier dbaf4aa evidence remains separate: five focused native tests passed; a GCC15 full-core run stopped on an Arrow17 timestamp-conversion overflow. Sixteen native-reader-to-cuDF GPU cases matched data but failed leak detection; a cuDF-only control reproduced the report. These do not establish clean sanitizer or Sirius SQL E2E qualification.

API and documentation

Interfaces and formats remain unchanged. The custom pool controls only allocations routed through it. Callers must handle exceptions and failed Results and retain the pool while its allocations are in use. A pool cap is not a total native-memory bound.

Generative AI tooling

Generated-by: OpenAI Codex (GPT-6)

[realAaronWu]

@lucasfang all four upstream checks now pass on unchanged head f6acf07c3597fe56377d10d275f837d817138e5a:

• Build and Test: all twelve native matrix jobs plus script-tests passed.
• GCC8, pre-commit and License passed.

Independent checks verified the tested PR merge has the reviewed head's exact tree, actual job/step completion and selected native/GCC8/pre-commit raw logs. The description distinguishes complete logs from successful metadata where transport limited downloads, and retains the sanitizer/support boundaries. Local/fork qualification and source-review findings are closed; no new source change or CI rerun is proposed.

Could you review the PR and merge it if the implementation and documented scope look acceptable? My account has no upstream merge permission. This updates the original approval request; CI approval is resolved. The dependent duckdb-paimon #145 also passes its final Linux SQL/distribution and Linux/macOS dynamic bridge CI, while its partial experimental scope remains separately tracked.

[realAaronWu]

@lucasfang gentle follow-up: could you review this PR and approve/merge if the implementation and documented scope look good? Unchanged head f6acf07c now has all upstream checks passing, including all 12 native matrix jobs, GCC8, script-tests, pre-commit and license checks. The implementation and validation details are in the description and comment above. This supplies finite planning budgets used by duckdb-paimon#145. Thanks!

[lxy-9602]

Thank you for your submission! We’ve been preparing for the 0.4 release recently and there’s a fair amount of regression work to work through. I’m afraid we may not be able to start the review this week, but we expect to begin next week. Thanks again for your patience and understanding!

[lxy-9602]

@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.

[realAaronWu]

Thanks for the suggestion. I've narrowed the PR to reuse the existing planning path:

• Removed the separate planner/metadata decoder and STL-specific allocation accounting; callers use WithMemoryPool().
• Fixed exception cleanup so accepted manifest tasks finish before planning unwinds, preserving the original failure and preventing tasks from outliving borrowed scan/snapshot state.
• Documented that accounting covers pool-managed allocations only, along with the caller's pool lifetime and thread-safety responsibilities.

Added focused regression tests for allocation refusal, cleanup, retained splits, and asynchronous failure paths.

The PR remains Draft pending current-head CI.

[lucasfang]

+1

[lxy-9602]

+1. Quick reminder: if you think the upcoming PR changes are reviewable, you can remove the draft marker. Regardless of whether CI passes, we’ll review all PRs that are marked ready and trigger CI.