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

fix(to_sql): use default backend for sql generation when set #9228

Merged
merged 1 commit into from
May 21, 2024

Conversation

gforsyth
Copy link
Member

We weren't passing use_default=True when looking for backends in
to_sql when no dialect was specified. This worked for nearly all
cases, except when an expression was built from a memtable.
A memtable doesn't have a specific backend, but if a user has set a
default backend, we should assume that dialect for rendering
sql (instead of our default of DuckDB sql)

Resolves #9227

@gforsyth
Copy link
Member Author

right, snapshot test with a generated name... sigh.

@gforsyth gforsyth closed this May 21, 2024
@gforsyth
Copy link
Member Author

going to regenerate snapshots with a static name, just wanted to kill the CI jobs that hadn't failed yet.

Copy link
Member

@jcrist jcrist left a comment

Choose a reason for hiding this comment

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

Nice find!

ibis/backends/tests/test_sql.py Outdated Show resolved Hide resolved
@gforsyth gforsyth reopened this May 21, 2024
@gforsyth gforsyth closed this May 21, 2024
We weren't passing `use_default=True` when looking for backends in
`to_sql` when no `dialect` was specified.  This worked for nearly all
cases, except when an expression was built from a `memtable`.
A `memtable` doesn't have a specific backend, but if a user has set a
default backend, we should assume that dialect for rendering
sql (instead of our default of DuckDB sql)
@gforsyth gforsyth reopened this May 21, 2024
@gforsyth gforsyth enabled auto-merge (squash) May 21, 2024 21:52
@gforsyth gforsyth merged commit c66d6aa into ibis-project:main May 21, 2024
82 checks passed
@gforsyth gforsyth deleted the to_sql_defaults branch May 22, 2024 13:14
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.

bug: to_sql always shows DuckDB SQL for a memtable even if there's a default backend set
2 participants