[Bug]: DuckDB reserved words aren't accurately listed in IdentifierPreparer, leading to synax errors.

[asford]

What happened?

duckdb_engine inherits it's IdentifierPreparer from the postgres dialect, which defines a fixed set of reserved keywords.
Postgres's set of reserved keywords is slightly different than the sql standard and duckdb (ref), and has more non-reserved keywords.

DuckDB's reserved keyworlds are more expansive and are accessible under duckdb_keywords.

If a column name uses a duckdb-reserved but postgres-unreserved keyword as a column name, e.g. "show",
then sqlalchemy won't properly quote the identifier and a semi-opaque syntax error is returned.

This can be patched at runtime via:

duckdb_keywords = set(
    duckdb.execute(
        "select keyword_name from duckdb_keywords() where keyword_category == 'reserved'"
    ).df()["keyword_name"]
)

duckdb_engine.DuckDBIdentifierPreparer.reserved_words = duckdb_keyword

This could (should?) be moved into duckdb_engine.

DuckDB Engine Version

0.14.0

DuckDB Version

1.0

SQLAlchemy Version

1.4.41

Relevant log output

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[Mause]

Hey there, thanks for reporting this! I've released a fix in 0.14.2

Feel free to let me know if you find further issues!

[johnallen3d]

@Mause - I think this change is causing issues in applications using some form of concurrency. In our case we started experiencing crashes of Gunicorn workers in Superset (using stock configuration) when using duckdb-engine version 0.14.2.

apache/superset#31978 (comment)

[Mause]

Well that isn't good - do you have a code sample I could use to reproduce this @johnallen3d ?

[johnallen3d]

I have minimal Python experience @Mause but I've managed to recreate what I think is happening within these Superset/Gunicorn requests in a simple script. This causes crashes in the majority of execution:

import threading
from sqlalchemy import create_engine, text

_thread_local = threading.local()

def get_engine():
    if not hasattr(_thread_local, "engine"):
        _thread_local.engine = create_engine("duckdb:///:memory:")
    return _thread_local.engine

def query():
    engine = get_engine()
    with engine.connect() as conn:
        result = conn.execute(text("SELECT version();"))
        print(result.scalar())

threads = [threading.Thread(target=query) for _ in range(10)]

for t in threads:
    t.start()

for t in threads:
    t.join()

If I directly modify the code inside of my local duckdb-engine __init__.py to use duckdb.cursor().execute() then this script works fine. I'm not sure if cursor() is technically the correct usage though.

[asford]

Oof, sorry for the bad beta on that.

The duckdb docs on multi threaded operations specifically calls out that different threads need to use .cursor to open separate handles to the shared db.

https://duckdb.org/docs/guides/python/multiple_threads.html

So this solution is likely correct & safe. The content returned here doesn't depend on the database, so there's no problem with executing against the default db.