Skip to content

Use SQLAlchemy 2.0's type_compiler_cls and type_compiler_instance #895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laughingman7743 merged 2 commits into from
Sep 29, 2026
Merged

Use SQLAlchemy 2.0's type_compiler_cls and type_compiler_instance #895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e273459
Use SQLAlchemy 2.0's type_compiler_cls and type_compiler_instance
laughingman7743 Sep 29, 2026
fb82f4f
Merge branch 'master' into refactor/894-type-compiler-cls
laughingman7743 Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pyathena/sqlalchemy/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import (
TYPE_CHECKING,
Any,
ClassVar,
cast,
)

Expand Down Expand Up @@ -143,7 +144,7 @@ class AthenaDialect(DefaultDialect):
preparer: type[IdentifierPreparer] = AthenaDMLIdentifierPreparer
statement_compiler: type[SQLCompiler] = AthenaStatementCompiler
ddl_compiler: type[DDLCompiler] = AthenaDDLCompiler
type_compiler: type[GenericTypeCompiler] = AthenaTypeCompiler
type_compiler_cls: ClassVar[type[GenericTypeCompiler]] = AthenaTypeCompiler

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round one (behavior and implementation), by the authoring model; not an independent review.

Scope: base cf673c2, head e273459, all 3 changed files.

Result: CLEAN.

Checked:

  • SQLAlchemy 2.0.46 DefaultDialect.__init__ resolves legacy_tt_callable = getattr(self, 'type_compiler', None) before type_compiler_cls. No class in the dialect hierarchy defines type_compiler any more (git grep finds it only in the new test). The test asserts this for AthenaDialect and AthenaAioDialect, and the pandas, arrow, polars, s3fs, and rest dialects inherit from them.
  • type_compiler_instance and the instance-level type_compiler are the same object (default.py: self.type_compiler_instance = self.type_compiler = tt_callable(self)), so get_column_specification renders the same DDL. On real Athena, 59 create_table and dialect tests passed (sync and aio).
  • ClassVar[type[GenericTypeCompiler]] matches SQLAlchemy's ClassVar[Type[TypeCompiler]] declaration; mypy passes.
  • Regression coverage: the new test fails on the old code (2 failed) and passes on the new code (2 passed).

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review, relayed. The reviewer was Codex CLI 0.157.1 (codex exec --sandbox read-only), a different model from the author.

Scope: the full diff at e273459 in a detached snapshot. The prompt did not include the PR number, PR body, commit message, or earlier findings. Static review only: no builds or tests.

Result: CLEAN. "No actionable findings in the specified diff."

Covered: SQLAlchemy 2.0.46's DefaultDialect compiler selection and instance initialization; all registered sync and async dialects (rest, pandas, arrow, polars, s3fs); DDL type rendering; typing; legacy type_compiler overrides in third-party subclasses; and the new test alongside the existing compiler tests.

default_paramstyle: str = pyathena.paramstyle
max_identifier_length: int = 255
cte_follows_insert: bool = True
Expand Down
2 changes: 1 addition & 1 deletion pyathena/sqlalchemy/compiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ def get_column_specification(self, column: Column[Any], **kwargs) -> str:
type_ = "INT"
else:
# type_expression marks column DDL so STRUCT and MAP use Hive syntax.
type_ = self.dialect.type_compiler.process(column.type, type_expression=column)
type_ = self.dialect.type_compiler_instance.process(column.type, type_expression=column)

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round two (claims, callers, and operations), by the authoring model; not an independent review.

Scope: the same base and head, plus the PR body, the commit message, and #894.

Result: CLEAN.

Claims checked:

Callers:

  • No PyAthena code or docs read the class-level type_compiler.
  • A third-party subclass that assigns type_compiler still wins through SQLAlchemy's legacy branch.
  • The only break is a class-level read of AthenaDialect.type_compiler, which the PR body lists for the 4.0.0 release notes.

Operations: no AWS behavior changes; this changes only how the dialect selects its type compiler.

text = [f"{self.preparer.format_column(column)} {type_}"]
if column.comment:
text.append(f"{self._get_comment_specification(column.comment)}")
Expand Down
10 changes: 10 additions & 0 deletions tests/pyathena/sqlalchemy/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
from sqlalchemy.sql.schema import Column, MetaData, Table
from sqlalchemy.sql.selectable import TextualSelect

from pyathena.aio.sqlalchemy.base import AthenaAioDialect
from pyathena.converter import DefaultTypeConverter
from pyathena.cursor import Cursor
from pyathena.error import DatabaseError, OperationalError
from pyathena.formatter import DefaultParameterFormatter
from pyathena.sqlalchemy.base import AthenaDialect
from pyathena.sqlalchemy.compiler import AthenaTypeCompiler
from pyathena.sqlalchemy.types import (
TINYINT,
AthenaArray,
Expand Down Expand Up @@ -108,6 +110,14 @@ def close(self):


class TestAthenaDialect:
@pytest.mark.parametrize("dialect_class", [AthenaDialect, AthenaAioDialect])
def test_type_compiler(self, dialect_class):
# SQLAlchemy 2.0 builds the type compiler from type_compiler_cls. A legacy
# type_compiler class attribute would take precedence over it.
assert not hasattr(dialect_class, "type_compiler")
assert dialect_class.type_compiler_cls is AthenaTypeCompiler
assert isinstance(dialect_class().type_compiler_instance, AthenaTypeCompiler)

def test_columns_from_information_schema(self):
# Rows arrive unordered, and Athena reports a missing comment as NULL.
# The API cursor this path pins hands that over as None or as an empty
Expand Down
Loading