ploomber / jupysql

Better SQL in Jupyter. 📊
https://jupysql.ploomber.io
Apache License 2.0
689 stars 74 forks source link

more robust statement type checking #794

Open edublancas opened 1 year ago

edublancas commented 1 year ago

we're currently checking the statement type based on the first word: https://github.com/ploomber/jupysql/commit/564ac4c65d0cc0edc45777f24e52d7abc26a37e5#diff-ee061806ed7a4f5a79d8571f64241031f5c61240dbe59e548de0666a7996ab88R493-R497

this is because wen need to know if this is a select statement, a better approach would be to use sqlparse, because our current approach will fail when using CTEs + a non-select statement at the end:

In [2]: import sqlparse

In [3]: sqlparse.parse(query)[0].get_type()
Out[3]: 'SELECT'

In [4]: sqlparse.parse("with a as (select * from b) select * from b")[0].get_type()
Out[4]: 'SELECT'

In [5]: sqlparse.parse("with a as (select * from b) insert into product_log select * from b")[0].get_type()
Out[5]: 'INSERT'

In [6]: sqlparse.parse("from b")[0].get_type()
Out[6]: 'UNKNOWN'

In [7]:
edublancas commented 1 year ago

hey @anupam-tiwari: I see you opened a PR but this issue wasn't assigned to you.

in this case, it's fine since no one else is working on this, but you must follow the process:

anupam-tiwari commented 1 year ago

hey @anupam-tiwari: I see you opened a PR but this issue wasn't assigned to you.

in this case, it's fine since no one else is working on this, but you must follow the process:

  • request the issue to be assigned to you
  • write the acceptance criteria
  • start working on it

sorry about that, will keep that in mind for future contributions!