DevShiba commented on PR #24218:
URL: https://github.com/apache/datafusion/pull/24218#issuecomment-5343610291

   @kosiew Thanks for the detailed review — pushed both.
   
   **Cross-library FFI round trip**: added 
`test_ffi_execution_plan_byte_metrics_cross_library` in 
`datafusion/ffi/tests/ffi_execution_plan.rs`, following the same pattern as the 
existing `test_ffi_execution_plan_partition_statistics_cross_library`. It 
registers real `BytesCount`/`BytesGauge` metrics (via 
`MetricBuilder::bytes_counter`/`bytes_gauge`, the same production constructors) 
on a plan served through a separately loaded copy of the `datafusion_ffi` 
cdylib (new `create_exec_with_byte_metrics` factory + `ForeignLibraryModule` 
entry), then calls `.metrics()` on the resulting `ForeignExecutionPlan` — which 
crosses the real FFI vtable into that other library image — and asserts the 
rendered strings are still byte-formatted (`bytes_scanned{partition=0}=1536.0 
B`, `stream_memory_usage{partition=0}=2.0 KB`). This is a genuine ABI round 
trip, not the in-process `From` conversion the existing 
`physical_expr::metrics` tests cover.
   
   **`bytes_written` coverage**: added a 
`global_bytes_counter("bytes_written")` case to 
`test_bytes_counter_and_gauge_use_byte_units`, asserting `bytes_written=3.0 GB` 
— covers the un-partitioned builder path `ParquetSink` actually uses, alongside 
the existing partitioned `bytes_counter` coverage.
   
   Verified locally before pushing: the exact commands the three 
previously-failing CI jobs run, the exact `ci/scripts/rust_clippy.sh` script, 
`cargo fmt --check`, and the full `datafusion-ffi --features integration-tests` 
suite — all clean.


-- 
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]

Reply via email to