-
Notifications
You must be signed in to change notification settings - Fork 76
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
SqlMagic.autolimit doesn't add a limit clause to the query #1006
Comments
yeah, the effect of autolimit is not to add a you can see the relevant code here. the main reason not to add LIMIT, is that some DBs don't support it so we'd have to figure out a way to make it portable. I guess it wouldn't be too hard using sqlglot, but given that we support many databases, adding stuff like this ends up breaking features for some users. perhaps a more reasonable solution would be to add LIMIT only to the dbs that we're sure they support it, but even detecting which db the user is connected to is challenging and we've encountered edge cases. bottom line is that we've decided not to modify user-submitted queries to prevent these issues. if you have time, feel free to open a PR, perhaps there is another approach that I'm overlooking |
Ah i see, The doc mislead me. Thank you for the quick feedback. For the interactive analytics workload, we're also using Maybe i could use |
sounds good, thanks for your feedback! |
What happens?
We're using jupysql to query a trino cluster through jupyter notebook. I want to autolimit the query results that we query to the cluster, as it's intended to be used for an interactive analytics workload.
I expect the SqlMagic.autolimit to add a LIMIT clause to the query, but it doesn't, i can see the original query in the Trino Admin UI.
To Reproduce
trino_connection = create_engine("trino://trino_cluster....")
%config SqlMagic.autolimit=500
%sql trino_connection
%sql SELECT * FROM BIG_TABLE
SELECT * FROM BIG_TABLE
without the LIMIT ClauseOS:
Linux
JupySQL Version:
0.10.10
Full Name:
Takieddine KADIRI
Affiliation:
Société Générale Assurances
The text was updated successfully, but these errors were encountered: