Conversation
Add eight skipped test cases for the planned set_special_handling wrapper
around duckdb_scalar_function_set_special_handling (duckdb.h:3719).
The method marks a scalar function to receive NULL inputs directly,
bypassing DuckDB's default SQL NULL propagation that would otherwise
short-circuit the callback and return NULL for any NULL argument.
Tests cover:
- returns self for fluent chaining
- block receives nil for a single nullable parameter (coalesce pattern)
- block receives nil for any combination of two nullable parameters
- varargs + special handling with NULL coalescing
- non-NULL inputs still work correctly after set_special_handling
- block can explicitly return nil even with special handling enabled
- null_count function with INTEGER varargs — mirrors the canonical DuckDB
C API test (capi_scalar_functions.cpp) using concrete types:
my_null_count(40, 1, 3) → 0
my_null_count(1, 42, NULL) → 1
my_null_count(NULL, NULL, NULL) → 3
my_null_count() → 0
- null_count function with ANY varargs — exact C API test replica,
skipped until DuckDB::LogicalType::ANY is also exposed (issue #1122)
All tests are skipped with a message referencing issue #1122 until the
C extension method is implemented.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wraps duckdb_scalar_function_set_special_handling (duckdb.h:3719). By default DuckDB short-circuits scalar function callbacks and returns NULL whenever any input row contains a NULL argument. Calling set_special_handling disables that behaviour: the block is invoked even for NULL inputs, receiving nil for each NULL argument, so the function can implement its own NULL semantics (coalescing, counting NULLs, etc). Changes: - ext/duckdb/scalar_function.c: add thin C wrapper _set_special_handling (0-arg private method, same pattern as _set_varargs / _add_parameter) - lib/duckdb/scalar_function.rb: add public set_special_handling that calls _set_special_handling and returns self; add :any to SUPPORTED_TYPES so varargs_type= accepts DuckDB::LogicalType::ANY - lib/duckdb/converter/int_to_sym.rb: add 34 => :any to HASH_TYPES (the entry was simply missing, causing Unknown type: 34 errors) - test/duckdb_test/scalar_function_test.rb: activate all 8 skeleton tests added in the previous commit (all skips removed) Closes part of #1122. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When null_handling: true is passed, set_special_handling is called on
the function so the block receives nil for NULL inputs instead of DuckDB
short-circuiting and returning NULL.
sf = DuckDB::ScalarFunction.create(
name: :null_as_zero,
return_type: :integer,
parameter_type: :integer,
null_handling: true
) { |v| v.nil? ? 0 : v }
Defaults to false (standard SQL NULL propagation unchanged).
Tests added:
- default is false (NULL input still returns NULL)
- null_handling: true coalesces NULL to non-NULL
- null_handling: true works with varargs (null_count pattern)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds an opt-in null-handling mode to ScalarFunction (Ruby + C binding) so registered blocks receive Changes
Sequence DiagramsequenceDiagram
participant User as User Code
participant RubySF as DuckDB::ScalarFunction (Ruby)
participant CExt as ext/duckdb (C binding)
participant DuckDB as DuckDB Engine
participant Block as Ruby Block
rect rgba(220, 240, 220, 0.5)
Note over User,RubySF: default (null_handling: false)
User->>RubySF: .create(..., null_handling: false)
RubySF->>CExt: register_callback
User->>DuckDB: invoke with NULL
DuckDB->>DuckDB: short-circuit -> return NULL
DuckDB-->>User: NULL (block not called)
end
rect rgba(200, 220, 255, 0.5)
Note over User,RubySF: opt-in (null_handling: true)
User->>RubySF: .create(..., null_handling: true)
RubySF->>RubySF: set_special_handling
RubySF->>CExt: _set_special_handling -> duckdb_scalar_function_set_special_handling
RubySF->>CExt: register_callback
User->>DuckDB: invoke with NULL
DuckDB->>CExt: call registered callback
CExt->>Block: invoke Ruby block with nil for NULL args
Block-->>CExt: return custom result
CExt->>DuckDB: return result
DuckDB-->>User: custom result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/duckdb_test/scalar_function_test.rb (1)
1143-1164: Consider removing the stale skip intest_scalar_function_with_varargs_any_type.This test uses
DuckDB::LogicalType::ANYwithout a skip, which is correct now that this PR adds:anysupport. However, there's an existing test at line 865 (test_scalar_function_with_varargs_any_type) that still has a stale skip comment stating ANY type isn't exposed yet.While that test isn't part of this PR's changes, you may want to unskip it in a follow-up or within this PR to maintain test consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/duckdb_test/scalar_function_test.rb` around lines 1143 - 1164, The test suite contains a stale skip for ANY type in the test named test_scalar_function_with_varargs_any_type; remove the skip/skip comment that asserts DuckDB::LogicalType::ANY isn't exposed anymore (or change it to noop) so the test runs like test_set_special_handling_null_count_any_varargs; locate the test named test_scalar_function_with_varargs_any_type and delete or update the skip/skip-related conditional, then run the test suite to ensure it passes with the newly added :any support.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/duckdb_test/scalar_function_test.rb`:
- Around line 1143-1164: The test suite contains a stale skip for ANY type in
the test named test_scalar_function_with_varargs_any_type; remove the skip/skip
comment that asserts DuckDB::LogicalType::ANY isn't exposed anymore (or change
it to noop) so the test runs like
test_set_special_handling_null_count_any_varargs; locate the test named
test_scalar_function_with_varargs_any_type and delete or update the
skip/skip-related conditional, then run the test suite to ensure it passes with
the newly added :any support.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac36a1cc-23bf-427b-8634-b8a35f07687f
📒 Files selected for processing (5)
CHANGELOG.mdext/duckdb/scalar_function.clib/duckdb/converter/int_to_sym.rblib/duckdb/scalar_function.rbtest/duckdb_test/scalar_function_test.rb
DuckDB::LogicalType::ANY is now fully supported after adding 34 => :any to HASH_TYPES and :any to ScalarFunction::SUPPORTED_TYPES. Replace the skip with a count_args function that accepts ANY varargs and returns the number of arguments — confirming mixed-type calls work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Closes #1176 (sub-issue of #1122).
Wraps
duckdb_scalar_function_set_special_handlingso Ruby scalar functions can receivenilfor NULL inputs instead of DuckDB short-circuiting and returning NULL.Changes
ext/duckdb/scalar_function.c_set_special_handling(private, 0-arg) — same pattern as_set_varargslib/duckdb/scalar_function.rbset_special_handlingmethod (calls C wrapper, returnsselffor chaining)null_handling: falsekeyword toScalarFunction.create— whentrue, callsset_special_handlingautomatically:anytoSUPPORTED_TYPESsovarargs_type= DuckDB::LogicalType::ANYis acceptedlib/duckdb/converter/int_to_sym.rb34 => :anytoHASH_TYPES(was simply missing, causedUnknown type: 34errors)CHANGELOG.md# UnreleasedUsage
Behaviour
set_special_handlingset_special_handlingNULLnilTests
8 tests added covering:
selffor fluent chainingnilfor NULL inputs (single param, two params, varargs)nileven with special handlingANYvarargsScalarFunction.createwithnull_handling: false(default)ScalarFunction.createwithnull_handling: trueScalarFunction.createwithnull_handling: true+ varargsSummary by CodeRabbit
New Features
Tests