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 separate endpoint to fetch function names for autocomplete #12840

Merged
merged 8 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions superset-frontend/spec/javascripts/sqllab/SqlEditor_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ import ConnectedSouthPane from 'src/SqlLab/components/SouthPane';
import SqlEditor from 'src/SqlLab/components/SqlEditor';
import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar';
import { Dropdown } from 'src/common/components';
import { queryEditorSetSelectedText } from 'src/SqlLab/actions/sqlLab';
import {
queryEditorSetFunctionNames,
queryEditorSetSelectedText,
} from 'src/SqlLab/actions/sqlLab';

import { initialState, queries, table } from './fixtures';

Expand All @@ -45,7 +48,7 @@ const store = mockStore(initialState);

describe('SqlEditor', () => {
const mockedProps = {
actions: { queryEditorSetSelectedText },
actions: { queryEditorSetFunctionNames, queryEditorSetSelectedText },
database: {},
queryEditorId: initialState.sqlLab.queryEditors[0].id,
latestQuery: queries[0],
Expand Down
22 changes: 22 additions & 0 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export const QUERY_EDITOR_SET_QUERY_LIMIT = 'QUERY_EDITOR_SET_QUERY_LIMIT';
export const QUERY_EDITOR_SET_TEMPLATE_PARAMS =
'QUERY_EDITOR_SET_TEMPLATE_PARAMS';
export const QUERY_EDITOR_SET_SELECTED_TEXT = 'QUERY_EDITOR_SET_SELECTED_TEXT';
export const QUERY_EDITOR_SET_FUNCTION_NAMES =
'QUERY_EDITOR_SET_FUNCTION_NAMES';
export const QUERY_EDITOR_PERSIST_HEIGHT = 'QUERY_EDITOR_PERSIST_HEIGHT';
export const MIGRATE_QUERY_EDITOR = 'MIGRATE_QUERY_EDITOR';
export const MIGRATE_TAB_HISTORY = 'MIGRATE_TAB_HISTORY';
Expand Down Expand Up @@ -1300,3 +1302,23 @@ export function createCtasDatasource(vizOptions) {
});
};
}

export function queryEditorSetFunctionNames(queryEditor, dbId) {
return function (dispatch) {
return SupersetClient.get({
endpoint: encodeURI(`/api/v1/database/${dbId}/function_names/`),
})
.then(({ json }) =>
dispatch({
type: QUERY_EDITOR_SET_FUNCTION_NAMES,
queryEditor,
functionNames: json.function_names,
}),
)
.catch(() =>
dispatch(
addDangerToast(t('An error occurred while fetching function names')),
betodealmeida marked this conversation as resolved.
Show resolved Hide resolved
),
);
};
}
5 changes: 5 additions & 0 deletions superset-frontend/src/SqlLab/components/AceEditorWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type HotKey = {
interface Props {
actions: {
queryEditorSetSelectedText: (edit: any, text: null | string) => void;
queryEditorSetFunctionNames: (queryEditor: object, dbId: number) => void;
addTable: (queryEditor: any, value: any, schema: any) => void;
};
autocomplete: boolean;
Expand Down Expand Up @@ -85,6 +86,10 @@ class AceEditorWrapper extends React.PureComponent<Props, State> {
componentDidMount() {
// Making sure no text is selected from previous mount
this.props.actions.queryEditorSetSelectedText(this.props.queryEditor, null);
this.props.actions.queryEditorSetFunctionNames(
this.props.queryEditor,
this.props.queryEditor.dbId,
);
this.setAutoCompleter(this.props);
}

Expand Down
4 changes: 1 addition & 3 deletions superset-frontend/src/SqlLab/components/SqlEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,7 @@ class SqlEditor extends React.PureComponent {
sql={this.props.queryEditor.sql}
schemas={this.props.queryEditor.schemaOptions}
tables={this.props.queryEditor.tableOptions}
functionNames={
this.props.database ? this.props.database.function_names : []
}
functionNames={this.props.queryEditor.functionNames}
extendedTables={this.props.tables}
height={`${aceEditorHeight}px`}
hotkeys={hotkeys}
Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ export default class SqlEditorLeftBar extends React.PureComponent {

onDbChange(db) {
this.props.actions.queryEditorSetDb(this.props.queryEditor, db.id);
this.props.actions.queryEditorSetFunctionNames(
this.props.queryEditor,
db.id,
);
}

onTableChange(tableName, schemaName) {
Expand Down
2 changes: 2 additions & 0 deletions superset-frontend/src/SqlLab/reducers/getInitialState.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export default function getInitialState({
autorun: false,
templateParams: null,
dbId: defaultDbId,
functionNames: [],
queryLimit: common.conf.DEFAULT_SQLLAB_LIMIT,
validationResult: {
id: null,
Expand Down Expand Up @@ -80,6 +81,7 @@ export default function getInitialState({
autorun: activeTab.autorun,
templateParams: activeTab.template_params,
dbId: activeTab.database_id,
functionNames: [],
schema: activeTab.schema,
queryLimit: activeTab.query_limit,
validationResult: {
Expand Down
5 changes: 5 additions & 0 deletions superset-frontend/src/SqlLab/reducers/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,11 @@ export default function sqlLabReducer(state = {}, action) {
dbId: action.dbId,
});
},
[actions.QUERY_EDITOR_SET_FUNCTION_NAMES]() {
return alterInArr(state, 'queryEditors', action.queryEditor, {
functionNames: action.functionNames,
});
},
[actions.QUERY_EDITOR_SET_SCHEMA]() {
return alterInArr(state, 'queryEditors', action.queryEditor, {
schema: action.schema,
Expand Down
48 changes: 45 additions & 3 deletions superset/databases/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from superset.databases.filters import DatabaseFilter
from superset.databases.schemas import (
database_schemas_query_schema,
DatabaseFunctionNamesResponse,
DatabasePostSchema,
DatabasePutSchema,
DatabaseRelatedObjectsResponse,
Expand Down Expand Up @@ -83,6 +84,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"schemas",
"test_connection",
"related_objects",
"function_names",
}
resource_name = "database"
class_permission_name = "Database"
Expand Down Expand Up @@ -126,7 +128,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"explore_database_id",
"expose_in_sqllab",
"force_ctas_schema",
"function_names",
"id",
]
add_columns = [
Expand Down Expand Up @@ -170,6 +171,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
}
openapi_spec_tag = "Database"
openapi_spec_component_schemas = (
DatabaseFunctionNamesResponse,
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
TableMetadataResponseSchema,
Expand Down Expand Up @@ -642,8 +644,8 @@ def related_objects(self, pk: int) -> Response:
500:
$ref: '#/components/responses/500'
"""
dataset = DatabaseDAO.find_by_id(pk)
if not dataset:
database = DatabaseDAO.find_by_id(pk)
if not database:
return self.response_404()
data = DatabaseDAO.get_related_objects(pk)
charts = [
Expand Down Expand Up @@ -799,3 +801,43 @@ def import_(self) -> Response:
except DatabaseImportError as exc:
logger.exception("Import database failed")
return self.response_500(message=str(exc))

@expose("/<int:pk>/function_names/", methods=["GET"])
@protect()
@safe
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
f".function_names",
log_to_statsd=False,
)
def function_names(self, pk: int) -> Response:
"""Get function names supported by a database
---
get:
description:
Get function names supported by a database
parameters:
- in: path
name: pk
schema:
type: integer
responses:
200:
200:
description: Query result
content:
application/json:
schema:
$ref: "#/components/schemas/DatabaseFunctionNamesResponse"
401:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
500:
$ref: '#/components/responses/500'
"""
database = DatabaseDAO.find_by_id(pk)
if not database:
return self.response_404()
return self.response(200, function_names=database.function_names,)
6 changes: 5 additions & 1 deletion superset/databases/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,11 +413,15 @@ class DatabaseRelatedObjectsResponse(Schema):
dashboards = fields.Nested(DatabaseRelatedDashboards)


class DatabaseFunctionNamesResponse(Schema):
function_names = fields.List(fields.String())


class ImportV1DatabaseExtraSchema(Schema):
metadata_params = fields.Dict(keys=fields.Str(), values=fields.Raw())
engine_params = fields.Dict(keys=fields.Str(), values=fields.Raw())
metadata_cache_timeout = fields.Dict(keys=fields.Str(), values=fields.Integer())
schemas_allowed_for_csv_upload = fields.List(fields.String)
schemas_allowed_for_csv_upload = fields.List(fields.String())
Copy link
Member

Choose a reason for hiding this comment

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

should this change be here? Also how is fields.Str() different from fields.String()?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be String() here. Str is an alias of String (see bottom of https://marshmallow.readthedocs.io/en/stable/_modules/marshmallow/fields.html).

cost_estimate_enabled = fields.Boolean()


Expand Down
21 changes: 19 additions & 2 deletions tests/databases/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ def test_get_items(self):
"explore_database_id",
"expose_in_sqllab",
"force_ctas_schema",
"function_names",
"id",
]
self.assertGreater(response["count"], 0)
Expand Down Expand Up @@ -589,7 +588,8 @@ def test_info_security_database(self):
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert "can_function_names" in data["permissions"]
assert len(data["permissions"]) == 3

def test_get_invalid_database_table_metadata(self):
"""
Expand Down Expand Up @@ -1125,3 +1125,20 @@ def test_import_database_masked_password_provided(self):

db.session.delete(database)
db.session.commit()

@mock.patch("superset.db_engine_specs.base.BaseEngineSpec.get_function_names",)
def test_function_names(self, mock_get_function_names):
example_db = get_example_database()
if example_db.backend in {"hive", "presto"}:
return

mock_get_function_names.return_value = ["AVG", "MAX", "SUM"]

self.login(username="admin")
uri = "api/v1/database/1/function_names/"

rv = self.client.get(uri)
response = json.loads(rv.data.decode("utf-8"))

assert rv.status_code == 200
assert response == {"function_names": ["AVG", "MAX", "SUM"]}