ClickHouse / ClickHouse/ClickHouse

`isEnum(to_type)` check in `SetUtils.cpp` doesn't handle `Nullable(Enum)` wrapper

Open
#104,779 0 comments 0 reactions 1 assignee Claimed by @yakov-olkhovskiy View on GitHub
bug comp-datatype-wrapper comp-query-analyzer
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

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

_Retrospective finding from a historical scan of [PR #72686](https://github.com/ClickHouse/ClickHouse/pull/72686) (merged 2024-12-09). Confirmed on current codebase — close with a note if already fixed._

### Describe what's wrong

Using `IN` clause with unknown enum values on a `Nullable(Enum)` column throws `UNKNOWN_ELEMENT_OF_ENUM` exception, while the same query on a plain `Enum` column silently ignores the unknown value

**Root cause:** SetUtils.cpp:106: The condition `isEnum(to_type)` fails for Nullable(Enum) because isEnum() in IDataType.h:428 checks `idx == TypeIndex::Enum8 || idx == TypeIndex::Enum16`, but Nullable wrapper has TypeIndex::Nullable

**Why we believe this is a bug:** SetUtils.cpp:106 → `convertFieldToTypeCheckEnum` catches UNKNOWN_ELEMENT_OF_ENUM exception but only if `isEnum(to_type)` returns true. For `Nullable(Enum)`, `isEnum()` returns false because it checks TypeIndex which is `Nullable`, not `Enum8`/`Enum16`.

**Affected locations:**
- `src/Analyzer/SetUtils.cpp:106` — isEnum check in exception handler for unknown enum values

**Impact:** Users with Nullable(Enum) columns get unexpected exceptions when using IN clause with typos or unknown values, while the same query on plain Enum columns works. This breaks the consistency promise of the feature.

### Does it reproduce on most recent release?

Yes — confirmed on current `master` (commit `6d5199c8d0c`).

### How to reproduce

```sql
DROP TABLE IF EXISTS test_nullable_enum_unknown;
CREATE TABLE test_nullable_enum_unknown (
plain_enum Enum('a'=1, 'b'=2),
nullable_enum Nullable(Enum('a'=1, 'b'=2))
) ENGINE=Memory;
INSERT INTO test_nullable_enum_unknown VALUES ('a', 'a');

-- Plain Enum: unknown value 'c' should be silently ignored (returns 'a')
SELECT 'Plain Enum with unknown value:';
SELECT plain_enum FROM test_nullable_enum_unknown WHERE plain_enum IN ('a', 'c') ORDER BY plain_enum;

-- Nullable(Enum): should also silently ignore unknown value 'c' (BUG: throws UNKNOWN_ELEMENT_OF_ENUM)
SELECT 'Nullable(Enum) with unknown value:';
SELECT nullable_enum FROM test_nullable_enum_unknown WHERE nullable_enum IN ('a', 'c') ORDER BY nullable_enum;

DROP TABLE test_nullable_enum_unknown;
```

[Try it on ClickHouse Fiddle](https://fiddle.clickhouse.com/63125ee8-2edc-45f2-8c97-959158092a96)

### Expected behavior

```
Plain Enum with unknown value:
a
Nullable(Enum) with unknown value:
a
```

### Error message and/or stacktrace

```
Plain Enum with unknown value:
a
Nullable(Enum) with unknown value:
Received exception from server (version 26.5.1):
Code: 691. DB::Exception: Unknown element 'c' for enum, maybe you meant: ['a']: while converting 'c' to Enum8('a' = 1, 'b' = 2). (UNKNOWN_ELEMENT_OF_ENUM)
```

### Additional context

**Suggested fix:** Change `isEnum(to_type)` to `isEnum(removeNullable(to_type))` using the existing removeNullable helper from DataTypeNullable.h to handle the Nullable wrapper

**Analysis details:** Confidence HIGH | Severity P2 | Testability: `STATELESS_SQL`

Found during automated review of [PR #72686](https://github.com/ClickHouse/ClickHouse/pull/72686).

---
_ClickGapAI · Confidence: HIGH · Severity: P2 · Finding: `h_pr72686_001`_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.