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

DuckDB errors on round function in Siuba only #379

Closed
Alex-Monahan opened this issue Jan 19, 2022 · 5 comments
Closed

DuckDB errors on round function in Siuba only #379

Alex-Monahan opened this issue Jan 19, 2022 · 5 comments

Comments

@Alex-Monahan
Copy link

Alex-Monahan commented Jan 19, 2022

Hello!

For some reason, the query that siuba outputs as text runs correctly on DuckDB, but the siuba expression does not run. This example is in the examples-duckdb.ipynb file that is attached here.

Two commands are required to install duckdb and the duckdb SQLAlchemy driver prior to running these test cases:

pip install duckdb
pip install duckdb_engine

This works:

import pandas as pd
pd.read_sql("""SELECT addresses.id, addresses.user_id, addresses.email_address, round(CAST(addresses.id AS NUMERIC), 2) AS id2 
FROM addresses""",conn)

This fails:

(tbl_addresses
  >> mutate(id2 = _.id.round(2))
  >> show_query()
  >> collect()
)

with this message:

RuntimeError: Not implemented Error: ROUND(DECIMAL, INTEGER) with non-constant precision is not supported

examples-duckdb.zip

@machow machow added this to siuba Jan 19, 2022
@machow
Copy link
Owner

machow commented Jan 19, 2022

Hey--thanks for this and PR #378. I was able to run the failing query successfully with duckdb. Do you know what siuba version you're using?

I'm fixing this in the next couple hours (in #377), but siuba right now is releasing as alpha releases, which don't get pip installed by default:

pip install siuba==1.0.0a3
# should be 1.0.0a3
import siuba
siuba.__version__

Duckdb looks pretty easy to set up, so I can open a quick PR that runs it through siuba's full test suite..!

@Alex-Monahan
Copy link
Author

I bet the version is the difference! I'll check this evening or tomorrow morning.

Thank you for running it through the test suite! That would be awesome. Let me know if you run into anything!

@machow
Copy link
Owner

machow commented Jan 19, 2022

@Alex-Monahan, here's a draft PR that extends the postgresql dialect a bit to support duckdb :). Overall, it seems to be pretty straightforward, with just two small hitches..

  1. verbs like mutate(x = 1), where the result is just a literal, are object dtypes in pandas for some reason.
  2. I had to fork duckdb_engine and change the dialect name attribute to from postgresql to duckdb (since this is how siuba looks up dialects). (This is more of a siuba quirk)

#380

@Alex-Monahan
Copy link
Author

You are exactly right - it was the version of siuba that was the issue. Closing this issue since this is already fixed!

@Alex-Monahan Alex-Monahan moved this to Done in siuba Jan 20, 2022
@Alex-Monahan
Copy link
Author

Alex-Monahan commented Jan 20, 2022

@Alex-Monahan, here's a draft PR that extends the postgresql dialect a bit to support duckdb :). Overall, it seems to be pretty straightforward, with just two small hitches..

  1. verbs like mutate(x = 1), where the result is just a literal, are object dtypes in pandas for some reason.
  2. I had to fork duckdb_engine and change the dialect name attribute to from postgresql to duckdb (since this is how siuba looks up dialects). (This is more of a siuba quirk)

#380

Thank you for filing those reports upstream with duckdb_engine! I think it may be time for DuckDB to leave the nest and become its own SQLAlchemy dialect... ;-) It sounds like it's as easy as changing the name!

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

No branches or pull requests

2 participants