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

%sqlrender {name} generates invalid SQL code #657

Closed
edublancas opened this issue Jun 23, 2023 · 4 comments · Fixed by #673
Closed

%sqlrender {name} generates invalid SQL code #657

edublancas opened this issue Jun 23, 2023 · 4 comments · Fixed by #673
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@edublancas
Copy link

%sqlrender generates invalid SQL code when rendering a snippet that does not have dependencies.

In [23]: %%sql --save penguins
    ...: select * from penguins.csv
    ...:
    ...:
Running query in 'duckdb://'
Out[23]:
+---------+-----------+----------------+---------------+-------------------+-------------+--------+
| species |   island  | bill_length_mm | bill_depth_mm | flipper_length_mm | body_mass_g |  sex   |
+---------+-----------+----------------+---------------+-------------------+-------------+--------+
|  Adelie | Torgersen |      39.1      |      18.7     |        181        |     3750    |  MALE  |
|  Adelie | Torgersen |      39.5      |      17.4     |        186        |     3800    | FEMALE |
|  Adelie | Torgersen |      40.3      |      18.0     |        195        |     3250    | FEMALE |
|  Adelie | Torgersen |      None      |      None     |        None       |     None    |  None  |
|  Adelie | Torgersen |      36.7      |      19.3     |        193        |     3450    | FEMALE |
|  Adelie | Torgersen |      39.3      |      20.6     |        190        |     3650    |  MALE  |
|  Adelie | Torgersen |      38.9      |      17.8     |        181        |     3625    | FEMALE |
|  Adelie | Torgersen |      39.2      |      19.6     |        195        |     4675    |  MALE  |
|  Adelie | Torgersen |      34.1      |      18.1     |        193        |     3475    |  None  |
|  Adelie | Torgersen |      42.0      |      20.2     |        190        |     4250    |  None  |
+---------+-----------+----------------+---------------+-------------------+-------------+--------+

In [24]: q=%sqlrender penguins
/Users/eduardo/dev/jupysql/src/sql/magic.py:75: FutureWarning: '%sqlrender' will be deprecated soon, please use '%sqlcmd snippets penguins' instead. For documentation, follow this link : https://jupysql.ploomber.io/en/latest/api/magic-snippets.html#id1
  warnings.warn(

In [25]: print(q)
WITH
select * from penguins.csv


In [26]: from sql.store import store

In [27]: print(store["penguins"])
WITH
select * from penguins.csv

you can see that the output is:

WITH
select * from penguins.csv

but it should be:

select * from penguins.csv

the WITH keyword should only appear when the snippet has dependencies, since it has to build the CTE. but when it doesn't the WITH should not appear.

the problem is in the store.py implementation: https://github.com/ploomber/jupysql/blob/master/src/sql/store.py

@edublancas edublancas added bug Something isn't working good first issue Good for newcomers labels Jun 23, 2023
@bbeat2782 bbeat2782 self-assigned this Jun 26, 2023
@bbeat2782
Copy link

Sorry, I didn't know clicking the button will assign my instantly.

It seems like the comment I got here is related to this issue. If this isn't for interview, can I get this one assigned?

@edublancas
Copy link
Author

yes, please work on it!

@bbeat2782
Copy link

Acceptance Criteria

  1. WITH only shows up when rendering a snippet that has a dependency
  2. Modify test functions to reflect 1)

@edublancas
Copy link
Author

sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants