-
Notifications
You must be signed in to change notification settings - Fork 22
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
WITH clause doesn't work with SQLite #50
Comments
Hey @n87, thank you for opening the issue :)
https://github.com/jupyter-xeus/xeus-sql/blob/master/src/xeus_sql_interpreter.cpp#L295-L317 Here are some things I'm thinking could be the culprit for this issue:
I'm not sure, these are just some hints. If you or anyone want to tackle this issue I'm here to help. |
@marimeireles it looks like your snippet is just enough to explain the issue. You check that first token is one of (SELECT, DESC, DESCRIBE, SHOW). But SELECT queries can also start with Note that |
Hum! This is where my lack of knowledge in SQL attacks again! Yeah, seems a bit complicated. If I understand correctly this I can't tell if SOCI offers support for it, because they have no entries about Afterwards... I'd say the steps to fix this would be:
|
A more googlable term for WITH clause is Common Table Expression. It is like defining variables, but more closer equivalent is
Somewhat contrived SQL(ite) equivalent:
|
Just came across this issue. Wanted to use Jupyter to teach Common Table Expressions, but seems it doesnt work :(. Is it just a matter of whitelisting the missing Please could we just add this as a quick fix? 🙏 |
Hey @darkdreamingdan, this is not a simple issue, unfortunately. My explanation in the previous comment is a bit confusing but I think there are two parts of this issue:
I'll try to tackle it this week. Will keep you updated of any progress. Another thing you could try is opening an issue in SOCI and asking them about the possibility of including this feature. If they support it then I'll add it here, as it's very easy to :) |
Seeing same issue, so I came here to see if this was previously reported. And you're already working it! Thank you very, very much for these wonderful tools! :tracking: |
Sorry to bring up this topic again. I'm having pretty much the same use case as @darkdreamingdan. I'd love to use xeus-sql to teach students how to use SQL. It's pretty much perfect for my use-case, except for this issue. We'd like to teach them the benefit of structuring queries using I'm not having much knowledge about C++, but it seems to me, these lines here determine which kind of queries are displaying a result in the notebook, based on the first keyword. Syntactically the xeus-sql/src/xeus_sql_interpreter.cpp Lines 312 to 315 in 315057d
Let me know what you think! |
If I may drop into this issue after such a long time, I am a large fan of CTE myself, and thus would be very happy if they could be taught early using jupyter... I fear my understanding of the problem, and the way @marimeireles described it are a bit at odds. The following is my understanding: CTE do not modify anything outside of the current query, they only provide a named binding to a (sub) query that (afaik) is limited to varying types of SELECT queries. But this binding is without consequence outside of the current query. Thus the Queries: WITH
inner_query_a as (
SELECT
1 AS id,
5 AS value
),
inner_query_b as (
SELECT
1 AS id,
2 AS value
)
SELECT
inner_query_a.value * inner_query_b.value
FROM inner_query_a, inner_query_b
WHERE inner_query_a.id=inner_query_b.id; and SELECT
inner_query_a.value * inner_query_b.value
FROM
(SELECT 1 AS id, 5 AS value) AS inner_query_a,
(SELECT 1 AS id, 2 AS value) AS inner_query_b
WHERE
inner_query_a.id = inner_query_b.id; are completely identical. This is only a tool to better structure your query, thus there should not need to be any additional support from the SOCI/xeus-sql side, beside accepting that a query that starts with "WITH" can return a table. The only potential proplem I see is that it does not have to return a table. From a high-level "What does the sql query do?" Point of view, the CTE does not really matter. Thus the simple "first token" approach to checking if the query returns some kind of result set is not valid. And fixing it to correctly ignore the CTE amounts to writing a more or less complete grammar of SQL...as understood by the various backends...yikes... A question I would pose is, is this distinction needed after all? Due to the way "first line comments" are now allowed (as in commit adacbae ) this whitelisting approach is not really consistent anymore, or do I missunderstand anything? Would it perhaps be possible to instead change to a blacklisting approach, i.e. define some queries that are sure not to return a result set from scanning the first line, and handle that, and show the resultset of all others, even if that resultset might be empty? |
Tested on these queries in the cloud demo [1]:
It doesn't output anything, but also doesn't show errors
[1] https://mybinder.org/v2/gh/jupyter-xeus/xeus-sql/stable?urlpath=lab/tree/examples/XVega%20operations.ipynb
The text was updated successfully, but these errors were encountered: