Skip to content

fix: scan Enum8/Enum16 columns into integer destinations#1921

Open
polyglotAI-bot wants to merge 1 commit into
mainfrom
polyglot/enum-scan-int-destinations
Open

fix: scan Enum8/Enum16 columns into integer destinations#1921
polyglotAI-bot wants to merge 1 commit into
mainfrom
polyglot/enum-scan-int-destinations

Conversation

@polyglotAI-bot

Copy link
Copy Markdown
Collaborator

Description

Fixes #1918.

Enum8.ScanRow / Enum16.ScanRow only accepted *string / **string
destinations (plus sql.Scanner). Any integer destination fell through to a
ColumnConverterError, so reading an enum column into its underlying numeric
ordinal was impossible — even though the write side has accepted integer
values since #802 ("Support inserting Enums as int8/int16/int values"). This
restores read/write symmetry.

proto.Enum8 is int8-based and proto.Enum16 is int16-based, so the raw
value returned by col.col.Row(row) is the ordinal. The fix adds integer
destination cases that return it.

Changes

  • lib/column/enum8.goScanRow now handles *int8, *int16, *int32,
    *int64, *int (and the ** pointer-to-pointer variants). All are lossless
    widenings of the int8 base.
  • lib/column/enum16.goScanRow now handles *int16, *int32, *int64,
    *int (and ** variants). int8 is intentionally not offered for
    Enum16
    : an Enum16 ordinal can exceed the int8 range (e.g. 1000) and
    would silently truncate, so an int8 destination for Enum16 stays an
    explicit ColumnConverterError rather than corrupting data (consistent with
    the "hard to use incorrectly" design principle). Only signed integer types are
    added, matching the signed enum ordinals and the write side.
  • Nullable(Enum) inherits the fix automatically — Nullable.ScanRow
    delegates element scanning to the base Enum column's ScanRow.

Test

tests/issues/1918_test.go (native round-trip against a real server):

  • Enum8 scanned into every signed width (int8/int16/int32/int64/int).
  • Enum16 scanned into int16/int32/int64/int, using ordinal 1000 (outside
    int8 range) to prove the wider destinations are not truncated.
  • Negative ordinals (-5, -300) preserve their sign.
  • Pointer-to-pointer destinations (**int8 etc.).
  • Contrast/regression guards: string destinations still work unchanged, and
    Enum16 -> int8 remains an error (no silent truncation).
  • Nullable(Enum) into integer destinations (value scanned; NULL leaves a
    pointer destination nil).

Each integer subtest fails on main with
clickhouse [ScanRow]: converting Enum8 to *int8 is unsupported and passes
with this change; the surrounding existing enum unit and integration tests
continue to pass unchanged.

Known limitation / follow-up (out of scope here)

Array(Enum)[]int and Map(String, Enum)map[string]int are not
addressed by this PR: they use a different, reflection-based element-scan path
(Array.scanSlicesetJSONFieldValue, and Map.ScanRow's whole-type match),
not Enum.ScanRow. Neither has ever worked, neither is a regression, and fixing
them touches shared code used by all container element types — so it belongs in a
separate change (and, for Map, may be intentional). Kept this PR to one focused
fix.

Pre-PR validation gate

  • Deterministic repro confirmed (integer subtests fail on main, pass here)
  • Root cause documented above
  • Fix targets the root cause (the ScanRow type switch)
  • Test fails without fix, passes with fix
  • No existing tests broken (enum unit + integration suites pass)
  • Convention compliance verified per AGENTS.md (regression test in
    tests/issues/, pointer receivers, t.Cleanup for connections)
  • No public API break; CHANGELOG is release-time generated from PR labels

Enum8/Enum16 ScanRow only accepted *string/**string destinations, so
scanning an enum column into an integer (its underlying ordinal) returned
a ColumnConverterError, even though the write side already accepts
int8/int16/int values (PR #802). Add integer destination cases that
return the numeric ordinal:

  Enum8  -> *int8/*int16/*int32/*int64/*int (+ **... variants)
  Enum16 -> *int16/*int32/*int64/*int       (+ **... variants)

int8 is intentionally not offered for Enum16 because Enum16 ordinals can
exceed the int8 range and would silently truncate; that destination
remains an explicit ColumnConverterError. Nullable(Enum) inherits the fix
via Nullable.ScanRow delegation.

Fixes: #1918
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[backfill: ClickHouse/clickhouse-go] Enum columns cannot be scanned into integer destinations

1 participant