Skip to content

Fix exception in intDiv/intDivOrZero on arrays of nullable tuples#100895

Merged
Algunenano merged 1 commit intoClickHouse:masterfrom
Algunenano:fix-intdiv-array-nullable-tuple
Mar 28, 2026
Merged

Fix exception in intDiv/intDivOrZero on arrays of nullable tuples#100895
Algunenano merged 1 commit intoClickHouse:masterfrom
Algunenano:fix-intdiv-array-nullable-tuple

Conversation

@Algunenano
Copy link
Copy Markdown
Member

executeArrayWithNumericImpl in FunctionBinaryArithmetic passed the input array's element type as result_type to the inner executeImpl, instead of the result array's element type.

This was harmless when actual values existed (the tuple function ignores result_type and computes correct columns via elem_func->getResultType). But when all elements are NULL, IFunction::execute's default Nullable handling short-circuits and creates a default column using the passed result_type:

if (rows_without_nulls == 0)
    return result_type->createColumnConstWithDefaultValue(...)

With the wrong result_type (e.g. Nullable(Tuple(Float64, Float64)) instead of Nullable(Tuple(Int64, Int64))), this produced Float64 columns while serialization expected Int64.

Closes #100873

Changelog category (leave one):

  • Critical Bug Fix (crash, data loss, RBAC)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix exception in intDiv/intDivOrZero on arrays of nullable tuples, e.g. SELECT intDiv([divide((1, 2), ... AND NULL)], 2).

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

`executeArrayWithNumericImpl` passed the input array's element type as
`result_type` to the inner `executeImpl`, instead of the result array's
element type. When all elements are NULL, `IFunction::execute`'s default
Nullable handling short-circuits and creates a default column using the
passed `result_type` — producing Float64 columns while serialization
expected Int64.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 27, 2026

Workflow [PR], commit [93ad667]

Summary:

job_name test_name status info comment
Upgrade check (amd_release) failure
Changed MergeTree settings are not reflected in the settings changes history (see changed_merge_tree_settings.txt) FAIL cidb IGNORED
New MergeTree settings are not reflected in settings changes history (see new_merge_tree_settings.txt) FAIL cidb IGNORED

AI Review

Summary

This PR fixes a real type mismatch in executeArrayWithNumericImpl by passing the nested return element type to executeImpl for array/number operations, and adds a focused stateless regression test covering both NULL-only and non-NULL paths. I did not find correctness, safety, concurrency, or performance problems in the patch.

ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! labels Mar 27, 2026
Algunenano added a commit to Algunenano/ClickHouse that referenced this pull request Mar 27, 2026
Replace the shallow `getDataType() != getColumnType()` check (which only
compared top-level TypeIndex) with a recursive `columnMatchesType` that
validates nested types inside Array, Nullable, Tuple, and Map.

The shallow check missed mismatches like
`Array(Nullable(Tuple(Int64, Int64)))` vs
`Array(Nullable(Tuple(Float64, Float64)))` because both have top-level
TypeIndex::Array. The deeper check would have detected the bug fixed in
ClickHouse#100895 and provided a clear error message at the function boundary
instead of a cryptic `Bad cast from type ColumnVector<double> to
ColumnVector<long>` deep in serialization.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@antaljanosbenjamin antaljanosbenjamin self-assigned this Mar 27, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 27, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.10% +0.00%
Functions 24.50% 24.60% +0.10%
Branches 76.70% 76.70% +0.00%

Changed lines: 100.00% (5/5) · Uncovered code

Full report · Diff report

@Algunenano Algunenano added this pull request to the merge queue Mar 28, 2026
Merged via the queue into ClickHouse:master with commit 25ba302 Mar 28, 2026
151 of 153 checks passed
@Algunenano Algunenano deleted the fix-intdiv-array-nullable-tuple branch March 28, 2026 14:17
robot-clickhouse added a commit that referenced this pull request Mar 28, 2026
robot-clickhouse added a commit that referenced this pull request Mar 28, 2026
robot-clickhouse added a commit that referenced this pull request Mar 28, 2026
robot-clickhouse added a commit that referenced this pull request Mar 28, 2026
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 28, 2026
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Mar 28, 2026
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Mar 28, 2026
clickhouse-gh Bot added a commit that referenced this pull request Mar 28, 2026
Backport #100895 to 26.1: Fix exception in intDiv/intDivOrZero on arrays of nullable tuples
clickhouse-gh Bot added a commit that referenced this pull request Mar 28, 2026
Backport #100895 to 26.2: Fix exception in intDiv/intDivOrZero on arrays of nullable tuples
fm4v added a commit that referenced this pull request Mar 30, 2026
Backport #100895 to 26.3: Fix exception in intDiv/intDivOrZero on arrays of nullable tuples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-critical-bugfix pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical error: Bad cast from type A to B (STID: 2241-6960)

5 participants