You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "emecii (via GitHub)" <gi...@apache.org> on 2026/09/09 12:32:38 UTC

[PR] fix: preserve scalar UDF strictness across FFI [datafusion]

emecii opened a new pull request, #25109:
URL: https://github.com/apache/datafusion/pull/25109

   ## Which issue does this PR close?
   
   - Part of #22330 (`ScalarUDFImpl::is_strict` slice).
   
   ## Rationale for this change
   
   A scalar UDF provider can declare that it always returns NULL when any argument is NULL. Across the FFI boundary, that declaration is currently lost and the consumer reports the default `false`, so optimizer nullability and null-rejecting reasoning cannot use the provider's strictness metadata.
   
   ## What changes are included in this PR?
   
   - Add an unconditional `is_strict` function pointer to `FFI_ScalarUDF`.
   - Dispatch it to the producer's `ScalarUDFImpl` and forward it from `ForeignScalarUDF`.
   - Reuse the existing planning-property fixture to cover forced-foreign and real cross-library round trips.
   
   ## What is the testing strategy for this PR?
   
   - `cargo fmt --all -- --check`
   - `cargo test -p datafusion-ffi --features integration-tests` (122 unit tests and all FFI integration targets passed)
   - Focused forced-foreign regression test
   - Focused `dlopen`/cdylib regression test
   
   Local lint note: this checkout provides Rust 1.98 while the repository pins 1.97. The unmodified dependency graph triggers six new Rust 1.98 clippy lints before/around `datafusion-ffi`; allowing only those pre-existing lints lets `cargo clippy -p datafusion-ffi --all-targets --all-features -- -D warnings` complete successfully. The exact workspace all-features lint also stalled in native `aws-lc-sys`/`protobuf-src`/`snmalloc-sys` build scripts on this macOS host.
   
   ## Are there any user-facing changes?
   
   Yes. Foreign scalar UDFs now preserve strictness metadata. Adding the function pointer changes the `FFI_ScalarUDF` layout, so this PR requires the `api change` label and must not be backported to a patch-release branch.
   
   ## AI assistance
   
   AI-generated with Codex (GPT-5). The implementation follows the existing unconditional function-pointer pattern for defaulted trait methods. No implementation unknowns are currently known; the local lint/toolchain limitations are disclosed above.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "emecii (via GitHub)" <gi...@apache.org>.
emecii commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-5601890578

   @timsaucer, could you please add the required `api change` label and trigger CI when convenient?
   
   AI-generated with Codex (GPT-5).


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-5609005339

   ## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25109?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 81.80%. Comparing base ([`4048898`](https://app.codecov.io/gh/apache/datafusion/commit/40488988ad596c9b093ad60e1453430d803ce33c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`54f894e`](https://app.codecov.io/gh/apache/datafusion/commit/54f894ef8f8b4e4f03092d08c7d50a5da07dc673?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##             main   #25109      +/-   ##
   ==========================================
   - Coverage   81.80%   81.80%   -0.01%     
   ==========================================
     Files        1130     1130              
     Lines      417754   417765      +11     
     Branches   417754   417765      +11     
   ==========================================
     Hits       341754   341754              
   - Misses      55875    55881       +6     
   - Partials    20125    20130       +5     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25109?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "emecii (via GitHub)" <gi...@apache.org>.
emecii commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-5604622039

   Cleaned up the description and removed the local environment details. Thanks.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-6406187774

   Marked as draft after 14 days without activity: merge conflicts.
   
   Please address these items and mark **Ready for review** when ready.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-5602098070

   This looks very straight forward. Can you clean up the PR description, though? There's a bit of slop in it.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: preserve scalar UDF strictness across FFI [datafusion]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25109:
URL: https://github.com/apache/datafusion/pull/25109#issuecomment-5610738373

   <!-- semver-check-comment -->
   Thank you for opening this pull request!
   
   Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
   
   <details>
   <summary>Details</summary>
   
   ```
        Cloning apache/main
       Building datafusion-ffi v55.0.0 (current)
          Built [  63.252s] (current)
        Parsing datafusion-ffi v55.0.0 (current)
         Parsed [   0.062s] (current)
       Building datafusion-ffi v55.0.0 (baseline)
          Built [  55.548s] (baseline)
        Parsing datafusion-ffi v55.0.0 (baseline)
         Parsed [   0.063s] (baseline)
       Checking datafusion-ffi v55.0.0 -> v55.0.0 (no change; assume patch)
        Checked [   0.244s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip
   
   --- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---
   
   Description:
   A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
           ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
          impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron
   
   Failed in:
     field FFI_ScalarUDF.is_strict in /home/runner/work/datafusion/datafusion/datafusion/ffi/src/udf/mod.rs:135
   
        Summary semver requires new major version: 1 major and 0 minor checks failed
       Finished [ 120.656s] datafusion-ffi
   ```
   
   </details>


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org