search

Mause / duckdb_engine

SQLAlchemy driver for DuckDB
MIT License
348 stars 39 forks source link

[Bug]: TypeError: visit_struct() missing 1 required positional argument: 'identifier_preparer' #1138

Open NickCrews opened 3 hours ago

NickCrews commented 3 hours ago

What happened?

Not sure if this is a bug, or I am using something wrong.

In what I'm sure is a very rare use case, if you have a type of struct<..., array<struct>, ...>, then I get TypeError: visit_struct() missing 1 required positional argument: 'identifier_preparer'. I'm happy to submit a PR if you give me a pointer on where to go. Thank you!

repro:

import duckdb_engine.datatypes as sadt
import sqlalchemy as sa
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column

class Base(DeclarativeBase):
    pass

class Person(Base):
    __tablename__ = "people"

    id: Mapped[int] = mapped_column(sa.Integer, primary_key=True)
    contact_info: Mapped[dict | None] = mapped_column(
        sadt.Struct(
            {
                "addresses": sa.ARRAY(
                    sadt.Struct(
                        {
                            "street": sa.String,
                            "city": sa.String,
                        }
                    )
                ),
            }
        )
    )

eng = sa.create_engine("duckdb:///:memory:")
Person.metadata.create_all(eng)
Details ```python-traceback --------------------------------------------------------------------------- TypeError Traceback (most recent call last) Cell In[1], line 35 15 contact_info: Mapped[dict | None] = mapped_column( 16 sadt.Struct( 17 { (...) 30 ) 31 ) 34 eng = sa.create_engine(\"duckdb:///:memory:\") ---> 35 Person.metadata.create_all(eng) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/schema.py:5868, in MetaData.create_all(self, bind, tables, checkfirst) 5844 def create_all( 5845 self, 5846 bind: _CreateDropBind, 5847 tables: Optional[_typing_Sequence[Table]] = None, 5848 checkfirst: bool = True, 5849 ) -> None: 5850 \"\"\"Create all tables stored in this metadata. 5851 5852 Conditional by default, will not attempt to recreate tables already (...) 5866 5867 \"\"\" -> 5868 bind._run_ddl_visitor( 5869 ddl.SchemaGenerator, self, checkfirst=checkfirst, tables=tables 5870 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/engine/base.py:3253, in Engine._run_ddl_visitor(self, visitorcallable, element, **kwargs) 3246 def _run_ddl_visitor( 3247 self, 3248 visitorcallable: Type[Union[SchemaGenerator, SchemaDropper]], 3249 element: SchemaItem, 3250 **kwargs: Any, 3251 ) -> None: 3252 with self.begin() as conn: -> 3253 conn._run_ddl_visitor(visitorcallable, element, **kwargs) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/engine/base.py:2459, in Connection._run_ddl_visitor(self, visitorcallable, element, **kwargs) 2447 def _run_ddl_visitor( 2448 self, 2449 visitorcallable: Type[Union[SchemaGenerator, SchemaDropper]], 2450 element: SchemaItem, 2451 **kwargs: Any, 2452 ) -> None: 2453 \"\"\"run a DDL visitor. 2454 2455 This method is only here so that the MockConnection can change the 2456 options given to the visitor so that \"checkfirst\" is skipped. 2457 2458 \"\"\" -> 2459 visitorcallable(self.dialect, self, **kwargs).traverse_single(element) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/visitors.py:664, in ExternalTraversal.traverse_single(self, obj, **kw) 662 meth = getattr(v, \"visit_%s\" % obj.__visit_name__, None) 663 if meth: --> 664 return meth(obj, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py:918, in SchemaGenerator.visit_metadata(self, metadata) 916 for table, fkcs in collection: 917 if table is not None: --> 918 self.traverse_single( 919 table, 920 create_ok=True, 921 include_foreign_key_constraints=fkcs, 922 _is_metadata_operation=True, 923 ) 924 else: 925 for fkc in fkcs: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/visitors.py:664, in ExternalTraversal.traverse_single(self, obj, **kw) 662 meth = getattr(v, \"visit_%s\" % obj.__visit_name__, None) 663 if meth: --> 664 return meth(obj, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py:956, in SchemaGenerator.visit_table(self, table, create_ok, include_foreign_key_constraints, _is_metadata_operation) 947 if not self.dialect.supports_alter: 948 # e.g., don't omit any foreign key constraints 949 include_foreign_key_constraints = None 951 CreateTable( 952 table, 953 include_foreign_key_constraints=( 954 include_foreign_key_constraints 955 ), --> 956 )._invoke_with(self.connection) 958 if hasattr(table, \"indexes\"): 959 for index in table.indexes: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py:314, in ExecutableDDLElement._invoke_with(self, bind) 312 def _invoke_with(self, bind): 313 if self._should_execute(self.target, bind): --> 314 return bind.execute(self) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/engine/base.py:1418, in Connection.execute(self, statement, parameters, execution_options) 1416 raise exc.ObjectNotExecutableError(statement) from err 1417 else: -> 1418 return meth( 1419 self, 1420 distilled_parameters, 1421 execution_options or NO_OPTIONS, 1422 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py:180, in ExecutableDDLElement._execute_on_connection(self, connection, distilled_params, execution_options) 177 def _execute_on_connection( 178 self, connection, distilled_params, execution_options 179 ): --> 180 return connection._execute_ddl( 181 self, distilled_params, execution_options 182 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/engine/base.py:1526, in Connection._execute_ddl(self, ddl, distilled_parameters, execution_options) 1522 schema_translate_map = exec_opts.get(\"schema_translate_map\", None) 1524 dialect = self.dialect -> 1526 compiled = ddl.compile( 1527 dialect=dialect, schema_translate_map=schema_translate_map 1528 ) 1529 ret = self._execute_context( 1530 dialect, 1531 dialect.execution_ctx_cls._init_ddl, (...) 1535 compiled, 1536 ) 1537 if self._has_events or self.engine._has_events: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/elements.py:308, in CompilerElement.compile(self, bind, dialect, **kw) 303 url = util.preloaded.engine_url 304 dialect = url.URL.create( 305 self.stringify_dialect 306 ).get_dialect()() --> 308 return self._compiler(dialect, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/ddl.py:69, in BaseDDLElement._compiler(self, dialect, **kw) 65 def _compiler(self, dialect, **kw): 66 \"\"\"Return a compiler appropriate for this ClauseElement, given a 67 Dialect.\"\"\" ---> 69 return dialect.ddl_compiler(dialect, self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:870, in Compiled.__init__(self, dialect, statement, schema_translate_map, render_schema_translate, compile_kwargs) 868 assert isinstance(statement, Executable) 869 self.execution_options = statement._execution_options --> 870 self.string = self.process(self.statement, **compile_kwargs) 872 if render_schema_translate: 873 self.string = self.preparer._render_schema_translates( 874 self.string, schema_translate_map 875 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:915, in Compiled.process(self, obj, **kwargs) 914 def process(self, obj: Visitable, **kwargs: Any) -> str: --> 915 return obj._compiler_dispatch(self, **kwargs) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/visitors.py:141, in Visitable._generate_compiler_dispatch.._compiler_dispatch(self, visitor, **kw) 139 return visitor.visit_unsupported_compilation(self, err, **kw) # type: ignore # noqa: E501 140 else: --> 141 return meth(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:6647, in DDLCompiler.visit_create_table(self, create, **kw) 6645 column = create_column.element 6646 try: -> 6647 processed = self.process( 6648 create_column, first_pk=column.primary_key and not first_pk 6649 ) 6650 if processed is not None: 6651 text += separator File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:915, in Compiled.process(self, obj, **kwargs) 914 def process(self, obj: Visitable, **kwargs: Any) -> str: --> 915 return obj._compiler_dispatch(self, **kwargs) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/visitors.py:141, in Visitable._generate_compiler_dispatch.._compiler_dispatch(self, visitor, **kw) 139 return visitor.visit_unsupported_compilation(self, err, **kw) # type: ignore # noqa: E501 140 else: --> 141 return meth(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:6678, in DDLCompiler.visit_create_column(self, create, first_pk, **kw) 6675 if column.system: 6676 return None -> 6678 text = self.get_column_specification(column, first_pk=first_pk) 6679 const = \" \".join( 6680 self.process(constraint) for constraint in column.constraints 6681 ) 6682 if const: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/dialects/postgresql/base.py:2185, in PGDDLCompiler.get_column_specification(self, column, **kwargs) 2183 colspec += \" SERIAL\" 2184 else: -> 2185 colspec += \" \" + self.dialect.type_compiler_instance.process( 2186 column.type, 2187 type_expression=column, 2188 identifier_preparer=self.preparer, 2189 ) 2190 default = self.get_column_default_string(column) 2191 if default is not None: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:960, in TypeCompiler.process(self, type_, **kw) 955 if ( 956 type_._variant_mapping 957 and self.dialect.name in type_._variant_mapping 958 ): 959 type_ = type_._variant_mapping[self.dialect.name] --> 960 return type_._compiler_dispatch(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/ext/compiler.py:493, in compiles..decorate..(*arg, **kw) 487 existing.specs[\"default\"] = _wrap_existing_dispatch 489 # TODO: why is the lambda needed ? 490 setattr( 491 class_, 492 \"_compiler_dispatch\", --> 493 lambda *arg, **kw: existing(*arg, **kw), 494 ) 495 setattr(class_, \"_compiler_dispatcher\", existing) 497 if specs: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/ext/compiler.py:546, in _dispatcher.__call__(self, element, compiler, **kw) 543 arm_collection = [] 544 kw[\"add_to_result_map\"] = lambda *args: arm_collection.append(args) --> 546 expr = fn(element, compiler, **kw) 548 if arm: 549 if not arm_collection: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/duckdb_engine/datatypes.py:217, in visit_struct(instance, compiler, identifier_preparer, **kw) 210 @compiles(Struct, \"duckdb\") # type: ignore[misc] 211 def visit_struct( 212 instance: Struct, (...) 215 **kw: Any, 216 ) -> str: --> 217 return \"STRUCT\" + struct_or_union(instance, compiler, identifier_preparer) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/duckdb_engine/datatypes.py:239, in struct_or_union(instance, compiler, identifier_preparer) 236 if fields is None: 237 raise exc.CompileError(f\"DuckDB {repr(instance)} type requires fields\") 238 return \"({})\".format( --> 239 \", \".join( 240 \"{} {}\".format( 241 identifier_preparer.quote_identifier(key), process_type(value, compiler) 242 ) 243 for key, value in fields.items() 244 ) 245 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/duckdb_engine/datatypes.py:241, in (.0) 236 if fields is None: 237 raise exc.CompileError(f\"DuckDB {repr(instance)} type requires fields\") 238 return \"({})\".format( 239 \", \".join( 240 \"{} {}\".format( --> 241 identifier_preparer.quote_identifier(key), process_type(value, compiler) 242 ) 243 for key, value in fields.items() 244 ) 245 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/duckdb_engine/datatypes.py:252, in process_type(value, compiler) 248 def process_type( 249 value: typing.Union[TypeEngine, Type[TypeEngine]], 250 compiler: PGTypeCompiler, 251 ) -> str: --> 252 return compiler.process(type_api.to_instance(value)) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:960, in TypeCompiler.process(self, type_, **kw) 955 if ( 956 type_._variant_mapping 957 and self.dialect.name in type_._variant_mapping 958 ): 959 type_ = type_._variant_mapping[self.dialect.name] --> 960 return type_._compiler_dispatch(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/visitors.py:141, in Visitable._generate_compiler_dispatch.._compiler_dispatch(self, visitor, **kw) 139 return visitor.visit_unsupported_compilation(self, err, **kw) # type: ignore # noqa: E501 140 else: --> 141 return meth(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/dialects/postgresql/base.py:2695, in PGTypeCompiler.visit_ARRAY(self, type_, **kw) 2694 def visit_ARRAY(self, type_, **kw): -> 2695 inner = self.process(type_.item_type, **kw) 2696 return re.sub( 2697 r\"((?: COLLATE.*)?)$\", 2698 ( (...) 2706 count=1, 2707 ) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/sql/compiler.py:960, in TypeCompiler.process(self, type_, **kw) 955 if ( 956 type_._variant_mapping 957 and self.dialect.name in type_._variant_mapping 958 ): 959 type_ = type_._variant_mapping[self.dialect.name] --> 960 return type_._compiler_dispatch(self, **kw) File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/ext/compiler.py:493, in compiles..decorate..(*arg, **kw) 487 existing.specs[\"default\"] = _wrap_existing_dispatch 489 # TODO: why is the lambda needed ? 490 setattr( 491 class_, 492 \"_compiler_dispatch\", --> 493 lambda *arg, **kw: existing(*arg, **kw), 494 ) 495 setattr(class_, \"_compiler_dispatcher\", existing) 497 if specs: File ~/code/scg/atlas/.venv/lib/python3.11/site-packages/sqlalchemy/ext/compiler.py:546, in _dispatcher.__call__(self, element, compiler, **kw) 543 arm_collection = [] 544 kw[\"add_to_result_map\"] = lambda *args: arm_collection.append(args) --> 546 expr = fn(element, compiler, **kw) 548 if arm: 549 if not arm_collection: TypeError: visit_struct() missing 1 required positional argument: 'identifier_preparer' ```

DuckDB Engine Version

0.13.2

DuckDB Version

1.0.0

SQLAlchemy Version

2.0.35

Relevant log output

No response

Code of Conduct

Mause commented 3 hours ago

Definitely sounds like a bug to me! I would imagine the fix would be hereabouts https://github.com/Mause/duckdb_engine/blob/main/duckdb_engine/datatypes.py#L230 though can't say for certain without the stack trace

NickCrews commented 3 hours ago

Ah, I think I have a fix. PR incoming!

NickCrews commented 3 hours ago

@Mause I think that linked PR should be a pretty straightforward review. Thanks!