Skip to content

cast_keep_nullable fails to preserve nullability when target type is LowCardinality because canBeInsideNullable returns false for LC types #103485

@clickgapai

Description

@clickgapai

Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.

Retrospective finding from a historical scan of PR #95747 (merged 2026-02-02). Confirmed on current codebase — close with a note if already fixed.

Describe what's wrong

With cast_keep_nullable = 1, casting from any nullable source (both Nullable(T) and LowCardinality(Nullable(T))) to a LowCardinality(U) target throws CANNOT_INSERT_NULL_IN_ORDINARY_COLUMN on NULL values instead of producing LowCardinality(Nullable(U))

Root cause: CastOverloadResolver.cpp:176-177: type->canBeInsideNullable() returns false for LowCardinality types, preventing makeNullable from being called. Should use makeNullableOrLowCardinalityNullableSafe(type) (DataTypeNullable.cpp:186) which handles LC targets by wrapping the inner dictionary type in Nullable.

Why we believe this is a bug: CastOverloadResolver.cpp:174-177 → getReturnTypeImpl checks type->canBeInsideNullable() before calling makeNullable(type). DataTypeLowCardinality inherits the default canBeInsideNullable() which returns false (IDataType.h:325). So the nullable wrapping is skipped for LC targets, and the return type stays LowCardinality(U) without nullability.

Affected locations:

  • src/Functions/CastOverloadResolver.cpp:176 — canBeInsideNullable guard blocks LC target nullability preservation
  • src/Functions/CastOverloadResolver.cpp:177 — makeNullable cannot create LowCardinality(Nullable(T)) — only Nullable(LowCardinality(T)) which is invalid

Impact: Any user with cast_keep_nullable = 1 casting nullable data to a LowCardinality target type gets an exception. This is the same class of bug that PR #95747 intended to fix but only addressed for non-LC targets.

Does it reproduce on most recent release?

Yes — confirmed on current master (commit 596eb6de5409).

How to reproduce

-- Test: cast_keep_nullable should preserve nullability when target type is LowCardinality
-- Bug: makeNullable + canBeInsideNullable guard fails for LowCardinality targets
-- because LowCardinality.canBeInsideNullable() returns false. Should use
-- makeNullableOrLowCardinalityNullable to get LowCardinality(Nullable(T)) instead.

SET cast_keep_nullable = 1;

-- Baseline: these work correctly (PR fix for non-LC targets)
SELECT NULL::Nullable(String)::String AS val, toTypeName(val) ORDER BY val;
SELECT NULL::LowCardinality(Nullable(String))::String AS val, toTypeName(val) ORDER BY val;

-- Bug: Nullable source to LowCardinality target throws instead of preserving nullability
SELECT NULL::Nullable(String)::LowCardinality(String) AS val, toTypeName(val) ORDER BY val;

-- Bug: LowCardinality(Nullable) source to LowCardinality target also throws
SELECT NULL::LowCardinality(Nullable(String))::LowCardinality(String) AS val, toTypeName(val) ORDER BY val;

-- Non-null should work regardless
SELECT 'hello'::LowCardinality(Nullable(String))::LowCardinality(String) AS val, toTypeName(val) ORDER BY val;

Try it on ClickHouse Fiddle

Expected behavior

\N	Nullable(String)
\N	Nullable(String)
\N	LowCardinality(Nullable(String))
\N	LowCardinality(Nullable(String))
hello	LowCardinality(Nullable(String))

Error message and/or stacktrace

\N	Nullable(String)
\N	Nullable(String)
Code: 349. DB::Exception: Cannot convert NULL value to non-Nullable type (CANNOT_INSERT_NULL_IN_ORDINARY_COLUMN)

Additional context

Open risks:

  • The accurateCastOrNull path (CastOverloadResolver.cpp:159-168) has a similar issue — it rejects LC targets entirely via validateNestedTypesForAccurateCastOrNull

Suggested fix: Replace lines 174-177 with: if (keep_nullable && (arguments.front().type->isNullable() || arguments.front().type->isLowCardinalityNullable() || isDynamic(*arguments.front().type))) return makeNullableOrLowCardinalityNullableSafe(type); — this uses the existing makeNullableOrLowCardinalityNullableSafe (DataTypeNullable.cpp:186) which handles both regular types and LC types correctly.

Analysis details: Confidence HIGH | Severity P1 | Testability: STATELESS_SQL

Found during automated review of PR #95747.


ClickGapAI · Confidence: HIGH · Severity: P1 · Finding: h_pr95747_001

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions