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.

Sign up for GitHub

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

Jump to bottom

fix(polars,mysql): avoid execution of query in _get_schema_using_query #9290

Merged
merged 2 commits into from
Jun 3, 2024
Merged

fix(polars,mysql): avoid execution of query in _get_schema_using_query #9290

merged 2 commits into from
Jun 3, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Jun 2, 2024

Closes #9162.

@cpcloud cpcloud changed the title avoid execution of query in dot sql fix(polars,mysql): avoid execution of query in dot sql Jun 2, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 2, 2024

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

@cpcloud cpcloud changed the title fix(polars,mysql): avoid execution of query in dot sql fix(polars,mysql): avoid execution of query in _get_schema_using_query Jun 2, 2024
@cpcloud cpcloud added this to the 9.1 milestone Jun 2, 2024
@cpcloud cpcloud added performance Issues related to ibis's performance mysql The MySQL backend polars The polars backend labels Jun 2, 2024
@cpcloud cpcloud force-pushed the avoid-execution-of-query-in-dot-sql branch 3 times, most recently from 7477018 to 65f0d80 Compare June 2, 2024 14:29
@cpcloud cpcloud force-pushed the avoid-execution-of-query-in-dot-sql branch from 65f0d80 to de8d6d3 Compare June 2, 2024 14:30
@cpcloud cpcloud requested a review from jcrist June 3, 2024 17:58
ncclementi
ncclementi approved these changes Jun 3, 2024
View reviewed changes
Copy link
Contributor

@ncclementi ncclementi left a comment

Choose a reason for hiding this comment

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

LGTM.

If possible, is there a way we can add test to make sure that if and when we add a new backend, we don't trigger execution?

@cpcloud
Copy link
Member Author

cpcloud commented Jun 3, 2024 •
edited
Loading

Hm, good question. I don't think so, since we often use arbitrary queries to access metadata and I don't think there's a way to easily determine whether such a query has only accessed metadata and not computed on the underlying data.

@cpcloud cpcloud merged commit 0348b9a into ibis-project:main Jun 3, 2024
74 checks passed
@cpcloud cpcloud deleted the avoid-execution-of-query-in-dot-sql branch June 3, 2024 19:07
cpcloud added a commit to cpcloud/ibis that referenced this pull request Jun 3, 2024
@cpcloud
fix(polars,mysql): avoid execution of query in `_get_schema_using_que…
360418b 
…ry` (ibis-project#9290)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncclementi ncclementi ncclementi approved these changes

@jcrist jcrist Awaiting requested review from jcrist

Assignees
No one assigned
Labels
mysql The MySQL backend performance Issues related to ibis's performance polars The polars backend
Projects
None yet
Milestone
9.1
Development

Successfully merging this pull request may close these issues.

bug: .sql() sometimes execute the query in order to get the schema
2 participants
@cpcloud @ncclementi