Skip to content

fix: stop Enum columns raising TypeError on _enums - #76

Open
TangoEnSkai wants to merge 1 commit into
databricks:mainfrom
TangoEnSkai:fix/enum-column-type-error
Open

fix: stop Enum columns raising TypeError on _enums#76
TangoEnSkai wants to merge 1 commit into
databricks:mainfrom
TangoEnSkai:fix/enum-column-type-error

Conversation

@TangoEnSkai

Copy link
Copy Markdown

Context

Any operation touching an Enum column fails:

TypeError: String.__init__() got an unexpected keyword argument '_enums'

Enum subclasses String, so with no colspecs entry of its own it resolves
through the String entry and SQLAlchemy adapts it to DatabricksStringType.
That class is a TypeDecorator, and TypeDecorator.__init__ passes its
arguments straight to its impl (String) — while Enum.adapt() forwards
Enum's internal keyword arguments. String rejects them.

metadata.create_all() on a model with an Enum column cannot run, as reported.

Worth noting where the crash actually lives: Enum("A", "B").compile(dialect)
succeeds and renders STRING. Only dialect_impl() — the path every real
query and DDL run takes — raises. That is why the existing camel_case_type_map
entry for Enum passes today without catching this.

What

  • Add DatabricksEnumType, a subclass of sqlalchemy.types.Enum that overrides
    only literal_processor() to escape literals the way DatabricksStringType
    does.
  • Register it in colspecs so Enum stops resolving through String.
  • Add TestDatabricksEnum covering the adaptation that used to crash, DDL
    compilation, native Python enum.Enum columns, literal escaping parity with
    String, length inference, validate_strings enforcement, and that ordinary
    String columns are unaffected.

Why

Two other approaches were tried first and rejected on evidence:

Filtering the offending keyword out is whack-a-mole. Dropping _enums just
moves the failure to _disable_warnings, and the set of forwarded keywords
belongs to SQLAlchemy, not to us.

Letting Enum fall back to SQLAlchemy's own implementation (mapping
Enumsqlalchemy.types.Enum) fixes the crash but changes literal rendering:

approach O'Bri\en renders as
String today (the target behaviour) 'O\'Bri\\en'
fall back to generic Enum 'O''Bri\en'
this PR 'O\'Bri\\en'

The middle row is precisely the single-quote doubling that DatabricksStringType
exists to prevent — fixing one bug by reintroducing another.

Subclassing Enum keeps every behaviour the generic type provides and changes
only the literal rendering. Checked against the SQLite dialect as a reference:

validate_strings=True, bad value default
SQLite (reference) LookupError passes through
this PR LookupError passes through

A TypeDecorator-based variant was also prototyped and discarded: it silently
lost validate_strings enforcement, letting "ZZZ" through where SQLite raises.

Completion Criteria

  • Enum columns adapt without TypeError, including the reported
    create_all() path
  • Enum literals escape identically to String literals
  • Enum validation, length inference and native enum.Enum support preserved,
    matching the SQLite dialect
  • Plain String columns unaffected
  • Removing only the colspecs entry fails exactly the four new tests that
    depend on it, and nothing else
  • pytest tests/test_local (offline modules) — 302 passed
  • black --check clean on all three changed files
  • CHANGELOG entry under a new # Unreleased section
  • Commit signed off (DCO)

Note

This touches base.py and CHANGELOG.md, which #74 also touches. The base.py
edits are far apart and merge cleanly; the CHANGELOG entries both open an
# Unreleased section, so whichever lands second needs a one-line rebase. Happy
to do that whenever you'd like.

close #61

Any operation touching an Enum column failed with

    TypeError: String.__init__() got an unexpected keyword argument '_enums'

Enum subclasses String, so with no colspecs entry of its own it resolved
through the String entry and SQLAlchemy adapted it to
DatabricksStringType. That class is a TypeDecorator whose __init__ hands
its arguments straight to its impl (String), while Enum.adapt() forwards
Enum's internal keyword arguments — so String received _enums and
rejected it. metadata.create_all() on a model with an Enum column could
not run.

Two approaches were tried and rejected before this one:

Filtering the offending keyword out is whack-a-mole. Dropping _enums
just moves the failure to _disable_warnings, and the set of forwarded
keywords is SQLAlchemy's to change.

Letting Enum fall back to SQLAlchemy's own implementation (mapping it to
sqlalchemy.types.Enum) fixes the crash but renders literals with doubled
single-quotes — 'O''Brien' rather than 'O\'Brien' — which is exactly the
breakage DatabricksStringType was written to prevent.

Subclassing Enum keeps every behaviour of the generic type (value
validation under validate_strings, length inference from the longest
value, native Python enum.Enum support) and overrides only
literal_processor, so Enum literals are escaped exactly as plain strings
are. Verified against the SQLite dialect as a reference: validation and
default behaviour now match it.

The existing camel_case_type_map covers Enum but did not catch this,
because it asserts on Enum(...).compile(dialect), which renders the type
name (STRING) without going through colspecs adaptation. Only
dialect_impl() — the path every real query and DDL run takes — hit the
crash. The new tests exercise that path.

Resolves databricks#61

Signed-off-by: TangoEnSkai <21152231+TangoEnSkai@users.noreply.github.com>
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.

TypeError when using Enum columns with the Databricks dialect (all 2.0.x releases)

1 participant