-
Notifications
You must be signed in to change notification settings - Fork 0
NEM-321 Sql query execution #112
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| from typing import Any | ||
|
|
||
| from databao_context_engine.datasources.datasource_discovery import get_datasource_descriptors, prepare_source | ||
| from databao_context_engine.datasources.sql_read_only import is_read_only_sql | ||
| from databao_context_engine.datasources.types import DatasourceId, PreparedConfig | ||
| from databao_context_engine.plugin_loader import DatabaoContextPluginLoader | ||
| from databao_context_engine.pluginlib.build_plugin import NotSupportedError, SqlRunnablePlugin | ||
| from databao_context_engine.pluginlib.plugin_utils import execute_sql_for_datasource | ||
| from databao_context_engine.pluginlib.sql.sql_types import SqlExecutionResult | ||
| from databao_context_engine.project.layout import ProjectLayout, logger | ||
|
|
||
|
|
||
| def run_sql( | ||
| project_layout: ProjectLayout, | ||
| datasource_id: DatasourceId, | ||
| sql: str, | ||
| params: list[Any] | None = None, | ||
| read_only: bool = True, | ||
| ) -> SqlExecutionResult: | ||
| if read_only and not is_read_only_sql(sql): | ||
| # we could use SqlReadOnlyDecision in the future | ||
| raise PermissionError("SQL execution is only supported for read-only queries") | ||
|
|
||
| logger.info(f"Running SQL query against datasource {datasource_id}: {sql}") | ||
| datasource_descriptor = get_datasource_descriptors(project_layout, [datasource_id])[0] | ||
|
|
||
| prepared = prepare_source(datasource_descriptor) | ||
| if not isinstance(prepared, PreparedConfig): | ||
| raise NotSupportedError("SQL execution is only supported for config-backed datasources") | ||
|
|
||
| loader = DatabaoContextPluginLoader() | ||
| plugin = loader.get_plugin_for_datasource_type(prepared.datasource_type) | ||
| if not isinstance(plugin, SqlRunnablePlugin): | ||
| raise NotSupportedError("Plugin doesn't support SQL execution") | ||
|
|
||
| return execute_sql_for_datasource( | ||
| plugin=plugin, | ||
| datasource_type=prepared.datasource_type, | ||
| config=prepared.config, | ||
| sql=sql, | ||
| params=params, | ||
| read_only=read_only, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| from dataclasses import dataclass | ||
| from enum import Enum | ||
|
|
||
|
|
||
| class SqlClass(Enum): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The naming sounds unclear. What is meant here? DatabaseAccess? DatabaseRole? |
||
| READ_ONLY = "read_only" | ||
| WRITE = "write" | ||
| UNKNOWN = "unknown" | ||
|
|
||
|
|
||
| @dataclass | ||
| class SqlReadOnlyDecision: | ||
| classification: SqlClass | ||
| reason: str | None = None | ||
|
|
||
|
|
||
| def classify_sql(sql: str) -> SqlReadOnlyDecision: | ||
| return SqlReadOnlyDecision(classification=SqlClass.READ_ONLY) | ||
|
|
||
|
|
||
| def is_read_only_sql(sql: str) -> bool: | ||
| # todo add sql parsing and allowlist of tokens here | ||
| return True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
| from io import BufferedReader | ||
| from typing import Any, Protocol, runtime_checkable | ||
|
|
||
| from databao_context_engine.pluginlib.sql.sql_types import SqlExecutionResult | ||
|
|
||
|
|
||
| @dataclass | ||
| class EmbeddableChunk: | ||
|
|
@@ -88,3 +90,23 @@ class DatasourceType: | |
| """ | ||
|
|
||
| full_type: str | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class SqlRunnablePlugin[T](BuildDatasourcePlugin[T], Protocol): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feel like it should be its own independent protocol rather than extending And plugins would keep on implementing I'm not fully sure how well it would work with the generics since we would want it to be the same for both the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or maybe it would be easier for the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried both those ways before and imo the implementation was less clear (there were issues with generics in the first case). but maybe it's worth trying somehow again |
||
| def run_sql( | ||
| self, | ||
| file_config: T, | ||
| sql: str, | ||
| params: list[Any] | None = None, | ||
| read_only: bool = True, | ||
| ) -> SqlExecutionResult: | ||
| """Execute SQL against the datasource represented by `file_config`. | ||
|
|
||
| Implementations should honor `read_only=True` by default and refuse mutating statements | ||
| unless explicitly allowed. | ||
|
|
||
| Raises: | ||
| NotSupportedError: If the plugin doesn't support this method. | ||
| """ | ||
| raise NotSupportedError("This method is not implemented for this plugin") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| from dataclasses import dataclass | ||
| from typing import Any | ||
|
|
||
|
|
||
| @dataclass | ||
| class SqlExecutionResult: | ||
| columns: list[str] | ||
| rows: list[tuple[Any, ...]] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| EmbeddableChunk, | ||
| ) | ||
| from databao_context_engine.pluginlib.config import ConfigPropertyAnnotation | ||
| from databao_context_engine.pluginlib.sql.sql_types import SqlExecutionResult | ||
| from databao_context_engine.plugins.databases.base_introspector import BaseIntrospector | ||
| from databao_context_engine.plugins.databases.database_chunker import build_database_chunks | ||
| from databao_context_engine.plugins.databases.introspection_scope import IntrospectionScope | ||
|
|
@@ -44,3 +45,13 @@ def check_connection(self, full_type: str, datasource_name: str, file_config: T) | |
|
|
||
| def divide_context_into_chunks(self, context: Any) -> list[EmbeddableChunk]: | ||
| return build_database_chunks(context) | ||
|
|
||
| def run_sql( | ||
| self, file_config: T, sql: str, params: list[Any] | None = None, read_only: bool = True | ||
| ) -> SqlExecutionResult: | ||
| return self._introspector.run_sql( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: This method doesn't really belong in the introspector I'm not sure it's worth it to take it out though: you'd need to refactor a bit to have a class executing SQL (including our SQL introspection queries) that is used by the Introspector |
||
| file_config=file_config, | ||
| sql=sql, | ||
| params=params, | ||
| read_only=read_only, | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.