fix(snowflake): properly pass schema and database for sqlglot generation #9221
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gforsyth
reviewed
May 21, 2024
There was a problem hiding this comment.
Thanks for putting this in @dlovell !
Small naming nit -- I just spent a long time eliminating the use of the word schema in the hierarchical sense (although I missed this bit, thanks for flagging it!). I think the fix here is good, I just want to make sure that grepping for schema across the codebase returns name->type mappings
Co-authored-by: Gil Forsyth <gforsyth@users.noreply.github.com>
dlovell
force-pushed
the
fix-snowflake-setup-session
branch
from
f4a89e1
to
018a0fd
Compare
|
Snowflake tests passing: We should probably have a test for this so we don't regress -- I'll try to sort that out in a followup |
|
Thanks for the fix @dlovell ! |
gforsyth
added a commit
to gforsyth/ibis
that referenced
this pull request
May 22, 2024
xref ibis-project#9221 We weren't handling passing in `catalog/database` as the `database` kwarg, despite our documentation telling users to do that. This was fixed in the above PR -- here I'm adding a (very gross) test to ensure that both connection methods work.
gforsyth
added a commit
to gforsyth/ibis
that referenced
this pull request
May 22, 2024
xref ibis-project#9221 We weren't handling passing in `catalog/database` as the `database` kwarg, despite our documentation telling users to do that. This was fixed in the above PR -- here I'm adding a (very gross) test to ensure that both connection methods work.
gforsyth
added a commit
that referenced
this pull request
May 22, 2024
xref #9221 We weren't handling passing in `catalog/database` as the `database` kwarg, despite our documentation telling users to do that. This was fixed in the above PR -- here I'm adding a test to ensure that both connection methods work.
mesejo
pushed a commit
to letsql/letsql
that referenced
this pull request
May 31, 2024
* test: add test of issues with ibis' snowflake connection as of 9.0.0 * fix(snowflake): monkeypatch snowflake setup * add a hotfix for proper snowflake connection initialization (pending next release that includes fix(snowflake): properly pass schema and database for sqlglot generation ibis-project/ibis#9221) * add a test for the expectation of initial snowflake connection state re catalog/database based on create_object_udfs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
use_stmtto prevent invalidUSE SCHEMAstatement whencurrently
use_stmtis generated like'USE SCHEMA "DATABASE/SCHEMA"', it should be'USE SCHEMA "DATABASE"."SCHEMA"'