Skip to content
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.

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

Already on GitHub? Sign in to your account

tests(sql): update failing test, due to duckdb change #441

Merged
merged 1 commit into from
Sep 16, 2022
Merged

Conversation

machow
Copy link
Owner

@machow machow commented Sep 16, 2022

Quick fix. A test (which is actual testing that an error is raised) began failing due to a change in exception types thrown by duckdb.

@machow machow marked this pull request as ready for review September 16, 2022 12:50
@machow machow merged commit 73f9a2f into main Sep 16, 2022
@machow machow deleted the fix-tests-duckdb branch September 16, 2022 12:50
@@ -38,7 +38,8 @@ def test_raw_sql_mutate_refer_previous_raise_dberror(backend, skip_backend, df):
if backend.name == "duckdb":
Copy link

Choose a reason for hiding this comment

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

With a new enough duckdb-engine version, this if clause shouldn't be required at all?

Copy link
Owner Author

@machow machow Sep 20, 2022

Choose a reason for hiding this comment

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

Ah, thanks for taking a look. I just checked with duckdb==0.5.1 and duckdb_engine==0.6.4, and see a sqlachemy.exc.ProgrammingError.

I'm realizing the BinderException is because I had to execute duckdb directly to get a DataFrame back efficiently :/ (due to this pandas issue: pandas-dev/pandas#45678)

I'll look into a bit more, but for duckdb, it looks like siuba runs this code:

siuba/siuba/sql/verbs.py

Lines 496 to 502 in 0d48e89

if _is_dialect_duckdb(__data.source):
# TODO: pandas read_sql is very slow with duckdb.
# see https://github.com/pandas-dev/pandas/issues/45678
# going to handle here for now. address once LazyTbl gets
# subclassed per backend.
duckdb_con = conn.connection.c
return duckdb_con.query(str(compiled)).to_df()

Copy link

Choose a reason for hiding this comment

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

Ah I see, that's unfortunate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants