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]