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

perf: should we be using con.sql(), not con.execute()? #8631

Closed
1 task done
NickCrews opened this issue Mar 12, 2024 · 1 comment · Fixed by #8669
Closed
1 task done

perf: should we be using con.sql(), not con.execute()? #8631

NickCrews opened this issue Mar 12, 2024 · 1 comment · Fixed by #8669
Assignees
Labels
feature Features or general enhancements

Comments

@NickCrews
Copy link
Contributor

Is your feature request related to a problem?

I was just reading through duckdb/duckdb#9710 (reply in thread), and in there Mytherin, one of the duckdb leads, mentions something about how python clients should prefer .sql() to .execute(). I don't quite understand the architecture of what he's saying, but I wanted to bring it to your attention. We use .execute(). Maybe it sounds like in duckdb>=0.10.0 there is no difference, but for our users who are on older duckdb versions, we should still switch to the performant way?

Sorry I'm not more useful here, I'm not sure what sort of benchmark to perform to test this out.

Describe the solution you'd like

NA

What version of ibis are you running?

main

What backend(s) are you using, if any?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jcrist
Copy link
Member

jcrist commented Mar 15, 2024

We actually do use con.sql(...).to_arrow_table() in the current release - this was added in #6875 (went out as part of 6.2, is still used in 8.0). In the main branch we've reverted back to using con.execute(...).fetch_arrow_table(). I tried rerunning some benchmarks and no longer see a measurable performance difference since duckdb 0.9.0 (on 0.8.0 there was a very noticeable difference for certain queries, but no longer). I've raised a question with the duckdb team to see if there's still a good reason to prefer con.sql over con.execute (if there is, then yes we should switch back), but for now I'm going to let this sit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Features or general enhancements
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants