Skip to content

fix: keep decimal and temporal types in nvl and ifnull - #25964

Open
1fanwang wants to merge 3 commits into
apache:mainfrom
1fanwang:1fanwang/fix-nvl-ifnull-type-coercion
Open

1fanwang wants to merge 3 commits into
apache:mainfrom
1fanwang:1fanwang/fix-nvl-ifnull-type-coercion

Conversation

@1fanwang

@1fanwang 1fanwang commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

nvl and ifnull lose decimal digits and convert dates and timestamps to strings, unlike coalesce. That also breaks timestamp arithmetic after ifnull. Their signature omits these types, so the planner coerces them to numbers or strings.

What changes are included in this PR?

The planner uses coalesce's type union for those types and retains existing coercion for primitive arguments and unsupported unions. This preserves accepted integer/boolean calls but also retains mixed-integer Float32 rounding. The shared helper validates argument counts; evaluation is unchanged.

What is the testing strategy for this PR?

SQL regressions cover decimal values, date and timestamp types, timestamp arithmetic and compatibility pairs. The public UDF and SQL tests also cover wrong argument counts.

Testing Done

Scenario Command Result
Decimal, date and timestamp results target/ci/datafusion-cli -f datafusion-25947.sql Values and types retained; timestamp arithmetic succeeds
Existing coercion target/ci/datafusion-cli -f nvl-kept-types.sql Unchanged
Argument count target/ci/datafusion-cli -q --format json -c 'SELECT nvl(1);' Standard arity error

datafusion-cli built with cargo build --profile ci -p datafusion-cli on main (416002a) and on this branch, running the issue's queries:

target/ci/datafusion-cli -f datafusion-25947.sql

On main, nvl returns Float64 for the decimals and Utf8View for the timestamp and date, and the interval addition fails:

DataFusion CLI v55.1.0
0 row(s) fetched. 
Elapsed 0.019 seconds.

+-----------------------+----------------------------------------+
| nvl_big               | coalesce_big                           |
+-----------------------+----------------------------------------+
| 1.2345678901234568e37 | 12345678901234567890123456789012345678 |
+-----------------------+----------------------------------------+
1 row(s) fetched. 
Elapsed 0.004 seconds.

+---------+----------+------------------+
| nvl_d   | ifnull_d | coalesce_d       |
+---------+----------+------------------+
| Float64 | Float64  | Decimal128(5, 2) |
+---------+----------+------------------+
1 row(s) fetched. 
Elapsed 0.003 seconds.

+----------+-----------+---------------+
| nvl_ts   | ifnull_ts | coalesce_ts   |
+----------+-----------+---------------+
| Utf8View | Utf8View  | Timestamp(ns) |
+----------+-----------+---------------+
1 row(s) fetched. 
Elapsed 0.003 seconds.

+----------+-------------+
| nvl_dt   | coalesce_dt |
+----------+-------------+
| Utf8View | Date32      |
+----------+-------------+
1 row(s) fetched. 
Elapsed 0.002 seconds.

Error during planning: Cannot coerce arithmetic expression Utf8View + Interval(MonthDayNano) to valid types

For these decimal, date and timestamp queries, nvl and ifnull retain the values and types, and timestamp arithmetic succeeds:

DataFusion CLI v55.1.0
0 row(s) fetched. 
Elapsed 0.152 seconds.

+----------------------------------------+----------------------------------------+
| nvl_big                                | coalesce_big                           |
+----------------------------------------+----------------------------------------+
| 12345678901234567890123456789012345678 | 12345678901234567890123456789012345678 |
+----------------------------------------+----------------------------------------+
1 row(s) fetched. 
Elapsed 0.019 seconds.

+------------------+------------------+------------------+
| nvl_d            | ifnull_d         | coalesce_d       |
+------------------+------------------+------------------+
| Decimal128(5, 2) | Decimal128(5, 2) | Decimal128(5, 2) |
+------------------+------------------+------------------+
1 row(s) fetched. 
Elapsed 0.005 seconds.

+---------------+---------------+---------------+
| nvl_ts        | ifnull_ts     | coalesce_ts   |
+---------------+---------------+---------------+
| Timestamp(ns) | Timestamp(ns) | Timestamp(ns) |
+---------------+---------------+---------------+
1 row(s) fetched. 
Elapsed 0.003 seconds.

+--------+-------------+
| nvl_dt | coalesce_dt |
+--------+-------------+
| Date32 | Date32      |
+--------+-------------+
1 row(s) fetched. 
Elapsed 0.002 seconds.

+---------------------+
| ifnull_plus         |
+---------------------+
| 2024-01-01T11:00:00 |
+---------------------+
1 row(s) fetched. 
Elapsed 0.006 seconds.

Argument types the signature already listed keep their result types, including pairs coalesce rejects or resolves differently, and a timestamp with a string still falls back to the original coercion:

target/ci/datafusion-cli -f nvl-kept-types.sql

On main:

DataFusion CLI v55.1.0
0 row(s) fetched. 
Elapsed 0.004 seconds.

+----------+----------+-------------+-----------+----------+
| int_bool | int_utf8 | int8_uint64 | null_null | ts_utf8  |
+----------+----------+-------------+-----------+----------+
| Utf8View | Utf8View | Float32     | Boolean   | Utf8View |
+----------+----------+-------------+-----------+----------+
1 row(s) fetched. 
Elapsed 0.004 seconds.

+-----------------+
| ifnull_int_bool |
+-----------------+
| 1               |
+-----------------+
1 row(s) fetched. 
Elapsed 0.001 seconds.

On this branch:

DataFusion CLI v55.1.0
0 row(s) fetched. 
Elapsed 0.005 seconds.

+----------+----------+-------------+-----------+----------+
| int_bool | int_utf8 | int8_uint64 | null_null | ts_utf8  |
+----------+----------+-------------+-----------+----------+
| Utf8View | Utf8View | Float32     | Boolean   | Utf8View |
+----------+----------+-------------+-----------+----------+
1 row(s) fetched. 
Elapsed 0.008 seconds.

+-----------------+
| ifnull_int_bool |
+-----------------+
| 1               |
+-----------------+
1 row(s) fetched. 
Elapsed 0.001 seconds.

The argument-count regressions now use the standard message:

target/ci/datafusion-cli -q --format json -c 'SELECT nvl(1);'
Error: Error during planning: Execution error: Function 'nvl' user-defined coercion failed with: Execution error: nvl function requires 2 arguments, got 1. No function matches the given name and argument types 'nvl(Int64)'. You might need to add explicit type casts.
	Candidate functions:
	nvl(UserDefined)
EXIT_CODE=1
Reproducer source: datafusion-25947.sql
CREATE TABLE t AS SELECT
  CAST('12345678901234567890123456789012345678' AS DECIMAL(38,0)) AS big,
  CAST(1.23 AS DECIMAL(5,2)) AS d,
  TIMESTAMP '2024-01-01 10:00:00' AS ts,
  DATE '2024-01-01' AS dt;

SELECT nvl(big, 0) AS nvl_big, coalesce(big, 0) AS coalesce_big FROM t;

SELECT
  arrow_typeof(nvl(d, d)) AS nvl_d,
  arrow_typeof(ifnull(d, d)) AS ifnull_d,
  arrow_typeof(coalesce(d, d)) AS coalesce_d
FROM t;

SELECT
  arrow_typeof(nvl(ts, ts)) AS nvl_ts,
  arrow_typeof(ifnull(ts, ts)) AS ifnull_ts,
  arrow_typeof(coalesce(ts, ts)) AS coalesce_ts
FROM t;

SELECT
  arrow_typeof(nvl(dt, dt)) AS nvl_dt,
  arrow_typeof(coalesce(dt, dt)) AS coalesce_dt
FROM t;

SELECT ifnull(ts, ts) + INTERVAL '1 hour' AS ifnull_plus FROM t;
Reproducer source: nvl-kept-types.sql
CREATE TABLE t AS SELECT TIMESTAMP '2024-01-01 10:00:00' AS ts;

SELECT
  arrow_typeof(nvl(arrow_cast(1, 'Int64'), false)) AS int_bool,
  arrow_typeof(nvl(arrow_cast(1, 'Int64'), arrow_cast('x', 'Utf8'))) AS int_utf8,
  arrow_typeof(nvl(arrow_cast(1, 'Int8'), arrow_cast(2, 'UInt64'))) AS int8_uint64,
  arrow_typeof(nvl(NULL, NULL)) AS null_null,
  arrow_typeof(nvl(ts, '2024-01-01T00:00:00')) AS ts_utf8
FROM t;

SELECT ifnull(arrow_cast(1, 'Int64'), false) AS ifnull_int_bool;

Are there any user-facing changes?

Yes. Decimal, date and timestamp results retain their types; existing primitive-type coercion remains unchanged.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Oct 2, 2026
coalesce rejects some pairs that nvl accepted, such as an integer and a
boolean, and resolves others differently. Arguments of the types the
original signature listed keep its coercion, and coalesce coercion only
applies to other types, falling back to the original when it fails.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.61%. Comparing base (416002a) to head (3829e6a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/functions/src/core/nvl.rs 90.32% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25964      +/-   ##
==========================================
+ Coverage   82.59%   82.61%   +0.01%     
==========================================
  Files        1145     1145              
  Lines      444096   444350     +254     
  Branches   444096   444350     +254     
==========================================
+ Hits       366806   367083     +277     
+ Misses      55021    54977      -44     
- Partials    22269    22290      +21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adriangb adriangb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @1fanwang, two small nits

Comment thread datafusion/functions/src/core/nvl.rs Outdated
Comment thread datafusion/functions/src/core/nvl.rs
@alamb

alamb commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

@1fanwang -- can you please help respond to @adriangb 's comments so we can merge this PR?

Thank you

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@1fanwang

Copy link
Copy Markdown
Contributor Author

can you please help respond to @adriangb 's comments so we can merge this PR?

@alamb I replied in both threads and pushed the shared-helper fix. Existing coercion stays compatible.

@adriangb

Copy link
Copy Markdown
Contributor

Thanks @1fanwang !

@adriangb
adriangb enabled auto-merge October 20, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nvl / ifnull convert decimals to Float64 and timestamps and dates to strings

4 participants