Repository navigation
Conversation
jaya232
force-pushed
the
fix/3304-enum-serialization-validation
branch
from
November 7, 2026 06:36
eabfd41 to
daa3d48
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3304
Problem
SerializerUtils.serializeEnumData()only validated theStringform of an Enum8/Enum16 value. Two other paths wrote data without checking:Numberwas written as-is, even if it is not a member of the column's enum (e.g.7intoEnum8('active' = 1, 'inactive' = 2))Enumwas written usingordinal(), which is the declaration index in Java and not the value declared in the column (Status.activeended up as0instead of1)The server accepts such rows, but reading them back as text then fails with
UNKNOWN_ELEMENT_OF_ENUM.Fix
Number: validated withClickHouseEnum.validate(int), so an unknown value fails on the client withUnknown enum value: 7Enum: mapped by name withClickHouseEnum.value(name()), so it is written as the declared value, and a constant that is not declared in the column failsTests
Added to
SerializerUtilsTest:The first, third and fourth tests failed before the change and pass now.
SerializerUtils*tests inclient-v2pass locally. The enum values in the existing integration tests (valid members, negative values and 0) still go through the same path, but I did not run the integration tests (no Docker).Compatibility
Behavior change: values that used to be written silently (non-member numbers, Java enums via
ordinal()) now throwIllegalArgumentException. Valid strings, valid member numbers and valid Java enum names are unchanged.Added
history/latest/3304.md.Note
Medium Risk
Behavior change on the insert path: code that relied on silent non-member numbers or Java enum ordinals will now throw at serialization time, though valid strings and declared member values are unchanged.
Overview
Enum8/Enum16 serialization in
SerializerUtils.serializeEnumData()now rejects invalid inserts on the client instead of writing bytes the server accepts but that break reads later.Numeric values go through
ClickHouseEnum.validate(int)so undeclared integers (e.g.7onEnum8('B' = 1, 'S' = 2)) throwIllegalArgumentExceptionwithUnknown enum value: …instead of being written raw.Java
Enumconstants are resolved by name viaClickHouseEnum.value(name()), so the wire value matches the column declaration (e.g.'active' = 1), not Javaordinal(). Constants whose names are not in the column definition also fail on insert.Unit tests in
SerializerUtilsTestcover invalid numbers, valid member numbers, Java enum mapping, and undeclared Java enum names. Changelog entry added inhistory/latest/3304.md.Reviewed by Cursor Bugbot for commit daa3d48. Bugbot is set up for automated code reviews on this repo. Configure here.