trinodb / trino-python-client

Python client for Trino
Apache License 2.0
328 stars 163 forks source link

typeof(uuid) returns incorrecd result `varchar(32)` when using sqlalchemy 2.0 #331

Closed cpcloud closed 1 year ago

cpcloud commented 1 year ago

Expected behavior

I expected a result of uuid

Actual behavior

I get a result of varchar(32)

Steps To Reproduce

In [13]: import sqlalchemy as sa

In [14]: engine = sa.create_engine("trino://user@localhost/memory/default")
<ipython-input-14-456274705007>:1: SADeprecationWarning: The dbapi() classmethod on dialect classes has been renamed to import_dbapi().  Implement an import_dbapi() classmethod directly on class <class 'trino.sqlalchemy.dialect.TrinoDialect'> to remove this warning; the old .dbapi() classmethod may be maintained for backwards compatibility.
  engine = sa.create_engine("trino://user@localhost/memory/default")

In [15]: with engine.begin() as con:
    ...:     print(con.exec_driver_sql("SELECT typeof(uuid())").fetchone())
    ...:
('uuid',)

In [16]: with engine.begin() as con:
    ...:     print(con.execute(sa.select(sa.func.typeof(sa.func.uuid()))).fetchone())
    ...:
('uuid',)

In [17]: import uuid

In [18]: with engine.begin() as con:
    ...:     print(con.execute(sa.select(sa.func.typeof(sa.literal(uuid.uuid4())))).fetchone())
    ...:
('varchar(32)',)

In [19]: with engine.begin() as con:
    ...:     print(con.execute(sa.select(sa.func.typeof(sa.literal(uuid.uuid4(), type_=sa.UUID())))).fetchone())
    ...:
('varchar(32)',)

In [20]: sa.__version__
Out[20]: '2.0.1'

In [23]: import trino

In [24]: trino.__version__
Out[24]: '0.321.0'

Log output

No response

Operating System

NixOS

Trino Python client version

0.321

Trino Server version

406

Python version

3.10.9

Are you willing to submit PR?

cpcloud commented 1 year ago

Running with SQLAlchemy 1.4.46 I get:

In [6]: with engine.begin() as con:
   ...:     print(con.execute(sa.select(sa.func.typeof(sa.literal(uuid.uuid4())))).fetchone())
   ...:
('uuid',)

when using a uuid.UUID object.

cpcloud commented 1 year ago

I suspect this is a result of the new sa.UUID type that was added in SQLAlchemy 2.0

mdesmet commented 1 year ago

@cpcloud : Can you provide a PR for this issue? Let me know how I can assist you!

cpcloud commented 1 year ago

@cpcloud : Can you provide a PR for this issue? Let me know how I can assist you!

No, sorry :) I've got too much on my plate as it is.

lpoulain commented 1 year ago

_format_prepared_param() in dbapi.py does handle the UUID type. The issue is that it receives a string. When looking at the stack trace:

trino/dbapi.py, line 396
trino/dbapi.py, line 387
trino/dbapi.py, line 490
trino/sqlalchemy/dialect.py, line 367
sqlalchemy/engine/base.py, line 1964
sqlalchemy/engine/base.py, line 1842
sqlalchemy/engine/base.py, line 1638
sqlalchemy/sql/elements.p", line 486
sqlalchemy/engine/base.py, line 1414

SQLAlchemy is calling the Python client's do_execute() method. The problem is that even at this level the parameter is a string. So the issue may be in the SQLAlchemy layer.