Skip to content

fix: protect SQLAlchemy reflection from String bytes format - #923

Closed
sankalpsthakur wants to merge 1 commit into
ClickHouse:mainfrom
sankalpsthakur:fix/920-sqlalchemy-internal-formats
Closed

fix: protect SQLAlchemy reflection from String bytes format#923
sankalpsthakur wants to merge 1 commit into
ClickHouse:mainfrom
sankalpsthakur:fix/920-sqlalchemy-internal-formats

Conversation

@sankalpsthakur

@sankalpsthakur sankalpsthakur commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #920.

set_default_formats("String", "bytes") (or set_read_format("String", "bytes")) is a global read format that makes String columns decode as bytes. The core driver already protects orchestration queries via _INTERNAL_QUERY_FORMATS = {"String": "string"}, but the SQLAlchemy reflection path (cc_sqlalchemy inspector + dialect metadata methods) issued ordinary DB-API queries and was unprotected.

Under the global bytes format this caused:

  1. LoudInspector.get_columns() / Table(..., autoload_with=...) raised TypeError: a bytes-like object is required, not 'str' on row.type.replace("\n", "").
  2. Silentget_table_names() returned [b'my_table'] instead of ['my_table'], and engine/comment comparisons like row.engine == "Dictionary" failed.

Changes

  • Thread per-query query_formats through dbapi.Cursor.execute / executemany and the SQLAlchemy dialect do_execute* methods, mirroring the existing settings / execution_options plumbing.
  • Apply the same _INTERNAL_QUERY_FORMATS override to all SQLAlchemy metadata queries (DESCRIBE TABLE, system.tables, SHOW TABLES / SHOW DATABASES, dictionary create SQL, etc.) via with_internal_query_formats(...).
  • User SELECTs are unchanged and still honor the global bytes format.

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

AI disclosure

This change was drafted with AI assistance and reviewed by the author before submission.

AI/LLM disclosure

  • AI coding tools (including Grok and/or Codex agent-assisted editing) were used to help draft or modify code and this PR description.
  • I reviewed the complete change, understand the reasoning, and ran the reported local tests before submitting.
  • This submission is original work of authorship under the project CLA / contributor terms; AI output was not pasted unreviewed.

set_default_formats("String", "bytes") previously broke reflection:
get_columns raised TypeError on bytes type names, and get_table_names
returned bytes table names. Thread query_formats through the DB-API
cursor and dialect (mirroring the existing settings path), and apply
the core driver's _INTERNAL_QUERY_FORMATS to metadata queries so schema
introspection always decodes String as str.

Fixes ClickHouse#920

AI disclosure: This change was drafted with AI assistance and reviewed
by the author before submission.
@sankalpsthakur

sankalpsthakur commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #935, same change rebased onto current main (resolves the cursor.py conflict with #927; keeps its _leading_keyword check and best-effort metadata probe while threading query_formats through). Closing this one; please review there.

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

Labels

None yet

Projects

None yet

1 participant