Error when using %sqlplot in snowflake #697
Conversation
tonykploomber
added
allow-workflow-edits
…ing-sqlplot-in-snowflake
…ing-sqlplot-in-snowflake
|
@edublancas |
There was a problem hiding this comment.
@tonykploomber I'm not sure I understood the solution.
Let's take the following table from our Snowflake connection
Since running %sqlplot histogram --table plot_something_e1bde5 -c y will raise the original error, I ran the following command:
It looks like our histogram is incorrect. It should be something like this
I went over the skipped tests artifacts and they look the same,
e.g.
expected:
results:
…ing-sqlplot-in-snowflake
src/sql/util.py
Outdated
| def to_upper_if_snowflake_conn(conn, upper): | ||
| return ( | ||
| upper.upper() | ||
| if isinstance(conn, AbstractConnection) |
There was a problem hiding this comment.
do we need to check if this is an abstract connection? do anything fails if we don't?
There was a problem hiding this comment.
Just wanna make sure there is callable function we can invoke
fix in 4a4f218
|
@tonykploomber this PR hasn't received any updates in two weeks. please address the review so we can merge it |
…ing-sqlplot-in-snowflake
|
@edublancas Apologize for missing updates Please check again |
src/sql/connection/__init__.py
Outdated
| @@ -11,6 +12,7 @@ | |||
|
|
|||
| __all__ = [ | |||
| "ConnectionManager", | |||
| "AbstractConnection", | |||
There was a problem hiding this comment.
do we need this import? I don't see it used anywhere
|
@tonykploomber fix the merge conflicts as well |
…ing-sqlplot-in-snowflake
|
@tonykploomber is this ready for review? if so, don't forget to request a review, otherwise I don't know if I review or wait |
|
@edublancas Preview: https://jupysql--697.org.readthedocs.build/en/697/integrations/snowflake.html#plotting |
Describe your changes
Fix %sqlplot histogram issue on snowflake
Original Problem - Table name and Column are uppercase
Actually our original internal histogram SQL query can perform the function
In the
test_sqlplot_histogram, we useplot_somethingdata to test. During the initial data loading stage, the table name and column are UPPERCASEWe will need to test against the uppercase table name and column name in the snowflake case
Solution
Make
test_sqlplot_histogramtest againstPLOT_SOMETHING_HASHKEYinstead ofplot_something_hashkeytable since it's created as uppercaseIssue number
Closes #444
Checklist before requesting a review
pkgmt format📚 Documentation preview 📚: https://jupysql--697.org.readthedocs.build/en/697/