Skip to content

FFI: get scalars from arrays - #8896

Open
myrrc wants to merge 2 commits into
developfrom
myrrc/ffi-getter-session
Open

FFI: get scalars from arrays#8896
myrrc wants to merge 2 commits into
developfrom
myrrc/ffi-getter-session

Conversation

@myrrc

@myrrc myrrc commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
  • Add vx_array_get_scalar method for retrieving vx_scalar.
  • Add scalar's primitive, utf8, bool, and binary getters generated from a macro.
  • Replace decimal scalar creation functions with a macro.
  • Remove vx_array's primitive getters in favor of obtaining a vx_scarar.
  • Add C++ API support for getting array's Scalar at index.
    Error propagation in C bindings #7483

@myrrc myrrc added the changelog/break A breaking API change label Jul 22, 2026
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from 3945f76 to c305a0e Compare July 22, 2026 12:32
@myrrc
myrrc requested a review from 0ax1 July 22, 2026 12:32
@myrrc myrrc changed the title Pass vx_session to ffi scalar getters" Pass vx_session to ffi scalar getters Jul 22, 2026
@myrrc myrrc added the lang/c Relates to the Vortex C (FFI) API label Jul 22, 2026
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from c305a0e to b1ac575 Compare July 22, 2026 12:34
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done b1ac575 1 Explore Profiling Data
Previous Runs (1)
Status Commit Job Attempt Link
🟢 Done 3945f76 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +2.5%
Engines: DataFusion Likely regression (+11.5%, medium confidence) · DuckDB No clear signal (-5.8%, low confidence)
Vortex (geomean): 0.954x ➖
Parquet (geomean): 0.942x ➖
Shifts: Parquet (control) -5.8% · Median polish -4.3%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (1.004x ➖, 0↑ 0↓)
name PR b1ac575 (ns) base 037b1a8 (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 9774512 10016685 0.98
vortex_q01/datafusion:vortex-file-compressed 6448746 6241940 1.03
datafusion / parquet (0.901x ➖, 1↑ 0↓)
name PR b1ac575 (ns) base 037b1a8 (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 20056827 21628324 0.93
vortex_q01/datafusion:parquet 🚀 4574698 5231454 0.87
duckdb / vortex-file-compressed (0.929x ➖, 0↑ 0↓)
name PR b1ac575 (ns) base 037b1a8 (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 10144766 10577988 0.96
vortex_q01/duckdb:vortex-file-compressed 5933116 6588883 0.90
duckdb / parquet (0.986x ➖, 0↑ 0↓)
name PR b1ac575 (ns) base 037b1a8 (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23406979 23594326 0.99
vortex_q01/duckdb:parquet 9363326 9549441 0.98

No file size changes detected.

@myrrc
myrrc enabled auto-merge (squash) July 22, 2026 12:36
Comment thread vortex-ffi/cinclude/vortex.h
Comment thread vortex-ffi/src/array.rs Outdated
@0ax1
0ax1 removed their request for review July 23, 2026 13:47
@myrrc myrrc changed the title Pass vx_session to ffi scalar getters ffi: separate scalar execution and primitive getters Jul 28, 2026
@myrrc myrrc changed the title ffi: separate scalar execution and primitive getters FFI: separate scalar execution and primitive getters Jul 28, 2026
@myrrc myrrc changed the title FFI: separate scalar execution and primitive getters FFI: separate scalar execution from primitive getters Jul 28, 2026
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from b1ac575 to ea86234 Compare July 28, 2026 14:13
@myrrc
myrrc requested a review from joseph-isaacs July 28, 2026 14:13
Comment thread vortex-ffi/src/scalar.rs
Comment thread vortex-ffi/src/scalar.rs Outdated
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from ea86234 to 255042c Compare July 29, 2026 14:50
@myrrc
myrrc requested a review from joseph-isaacs July 29, 2026 14:51
@myrrc
myrrc marked this pull request as draft July 29, 2026 14:56
auto-merge was automatically disabled July 29, 2026 14:56

Pull request was converted to draft

@myrrc

myrrc commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

I've checked again, and I think we want a more gradual change.

We do want to be able to get a scalar for an array, but we can't replace current getters with an intermediate vx_scalar:

  1. for bool, we would pay the price of allocating a scalar for every item. This is non-performant, but acceptable, and can be solved by ffi: create bool arrays #8899
  2. For strings, however, we can't have an intermediate Scalar because the vx_view's now borrow from a scalar, and C++ api's bulk access would become UB.

I've removed the primitive accessors here.

@myrrc myrrc changed the title FFI: separate scalar execution from primitive getters FFI: get scalars from arrays Jul 30, 2026
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from 255042c to 49f87e7 Compare July 30, 2026 15:10
@myrrc myrrc added changelog/feature A new feature changelog/break A breaking API change and removed changelog/break A breaking API change changelog/feature A new feature labels Jul 30, 2026
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch 2 times, most recently from 36789e7 to 11f23f4 Compare July 30, 2026 15:43
@myrrc
myrrc marked this pull request as ready for review July 30, 2026 15:55
@myrrc
myrrc enabled auto-merge (squash) July 30, 2026 15:55
@myrrc
myrrc requested review from joseph-isaacs and removed request for joseph-isaacs July 30, 2026 15:56
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from 11f23f4 to 413207b Compare July 30, 2026 15:58
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
@myrrc
myrrc force-pushed the myrrc/ffi-getter-session branch from 413207b to b0ff733 Compare July 30, 2026 16:03
@myrrc myrrc added the lang/cpp Relates to the Vortex C++ API label Jul 30, 2026
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change lang/c Relates to the Vortex C (FFI) API lang/cpp Relates to the Vortex C++ API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants