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

feat: add ability to connect to snowflake from core #7526

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

fontanierh
Copy link
Contributor

Description

Part 1 of #7450

  • adds ability to connect to a snowflake warehouse (from a JSON containing the connection details
  • adds a trait for remote databases
  • form now the trait has only 1 method: get_tables_used_by_query, which takes a query and returns a list of strings (fully qualified table identifiers)
  • add implementation for snowflake

Next PR: add ability to query the database and parse results into a Result<(Vec<QueryResult>, TableSchema), QueryDatabaseError>

Risk

N/A (not used yet)

Deploy Plan

N/A

})
.collect();

Ok(explain_rows)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit simple, is that working or is it just "stub" code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it works 🤷 Of course there will be a bit more code later, i.e check that those tables indeed map to tables we have synced and that the user making the query has the permission to use these tables, but I don't think there is a lot more to it

@fontanierh fontanierh merged commit c6d2ede into main Sep 19, 2024
3 checks passed
@fontanierh fontanierh deleted the feat/connect-to-snowflake-from-core branch September 19, 2024 13:42
Copy link
Contributor

@spolu spolu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants