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: implement create view as functionality in Sqllab #9794

Merged
merged 1 commit into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ assists people when migrating to a new version.

* [9786](https://github.com/apache/incubator-superset/pull/9786): with the upgrade of `werkzeug` from version `0.16.0` to `1.0.1`, the `werkzeug.contrib.cache` module has been moved to a standalone package [cachelib](https://pypi.org/project/cachelib/). For example, to import the `RedisCache` class, please use the following import: `from cachelib.redis import RedisCache`.

* [9572](https://github.com/apache/incubator-superset/pull/9572): a change which by defau;t means that the Jinja `current_user_id`, `current_username`, and `url_param` context calls no longer need to be wrapped via `cache_key_wrapper` in order to be included in the cache key. The `cache_key_wrapper` function should only be required for Jinja add-ons.
* [9794](https://github.com/apache/incubator-superset/pull/9794): introduces `create view as` functionality in the sqllab. This change will require the `query` table migration and potential service downtime as that table has quite some traffic.

* [9572](https://github.com/apache/incubator-superset/pull/9572): a change which by default means that the Jinja `current_user_id`, `current_username`, and `url_param` context calls no longer need to be wrapped via `cache_key_wrapper` in order to be included in the cache key. The `cache_key_wrapper` function should only be required for Jinja add-ons.

* [8867](https://github.com/apache/incubator-superset/pull/8867): a change which adds the `tmp_schema_name` column to the `query` table which requires locking the table. Given the `query` table is heavily used performance may be degraded during the migration. Scheduled downtime may be advised.

Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ipdb==0.12
isort==4.3.21
mypy==0.770
nose==1.3.7
parameterized==0.7.4
pip-tools==5.1.2
pre-commit==1.17.0
psycopg2-binary==2.8.5
Expand Down
6 changes: 6 additions & 0 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ export const addSuccessToast = addSuccessToastAction;
export const addDangerToast = addDangerToastAction;
export const addWarningToast = addWarningToastAction;

export const CtasEnum = {
TABLE: 'TABLE',
VIEW: 'VIEW',
};

// a map of SavedQuery field names to the different names used client-side,
// because for now making the names consistent is too complicated
// so it might as well only happen in one place
Expand Down Expand Up @@ -346,6 +351,7 @@ export function runQuery(query) {
tab: query.tab,
tmp_table_name: query.tempTableName,
select_as_cta: query.ctas,
ctas_method: query.ctas_method,
templateParams: query.templateParams,
queryLimit: query.queryLimit,
expand_data: true,
Expand Down
7 changes: 6 additions & 1 deletion superset-frontend/src/SqlLab/components/ResultSet.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import FilterableTable from '../../components/FilterableTable/FilterableTable';
import QueryStateLabel from './QueryStateLabel';
import CopyToClipboard from '../../components/CopyToClipboard';
import { prepareCopyToClipboardTabularData } from '../../utils/common';
import { CtasEnum } from '../actions/sqlLab';

const propTypes = {
actions: PropTypes.object,
Expand Down Expand Up @@ -219,10 +220,14 @@ export default class ResultSet extends React.PureComponent {
tmpTable = query.results.query.tempTable;
tmpSchema = query.results.query.tempSchema;
}
let object = 'Table';
if (query.ctas_method === CtasEnum.VIEW) {
object = 'View';
}
return (
<div>
<Alert bsStyle="info">
{t('Table')} [
{t(object)} [
<strong>
{tmpSchema}.{tmpTable}
</strong>
Expand Down
44 changes: 33 additions & 11 deletions superset-frontend/src/SqlLab/components/SqlEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
} from '../constants';
import RunQueryActionButton from './RunQueryActionButton';
import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import { CtasEnum } from '../actions/sqlLab';

const SQL_EDITOR_PADDING = 10;
const INITIAL_NORTH_PERCENT = 30;
Expand Down Expand Up @@ -284,7 +285,7 @@ class SqlEditor extends React.PureComponent {
this.startQuery();
}
}
startQuery(ctas = false) {
startQuery(ctas = false, ctas_method = CtasEnum.TABLE) {
const qe = this.props.queryEditor;
const query = {
dbId: qe.dbId,
Expand All @@ -299,6 +300,7 @@ class SqlEditor extends React.PureComponent {
? this.props.database.allow_run_async
: false,
ctas,
ctas_method,
updateTabState: !qe.selectedText,
};
this.props.actions.runQuery(query);
Expand All @@ -313,7 +315,10 @@ class SqlEditor extends React.PureComponent {
}
}
createTableAs() {
this.startQuery(true);
this.startQuery(true, CtasEnum.TABLE);
}
createViewAs() {
this.startQuery(true, CtasEnum.VIEW);
}
ctasChanged(event) {
this.setState({ ctas: event.target.value });
Expand Down Expand Up @@ -372,8 +377,13 @@ class SqlEditor extends React.PureComponent {
}
renderEditorBottomBar(hotkeys) {
let ctasControls;
if (this.props.database && this.props.database.allow_ctas) {
if (
this.props.database &&
(this.props.database.allow_ctas || this.props.database.allow_cvas)
) {
const ctasToolTip = t('Create table as with query results');
const cvasToolTip = t('Create view as with query results');

ctasControls = (
<FormGroup>
<InputGroup>
Expand All @@ -385,14 +395,26 @@ class SqlEditor extends React.PureComponent {
onChange={this.ctasChanged.bind(this)}
/>
<InputGroup.Button>
<Button
bsSize="small"
disabled={this.state.ctas.length === 0}
onClick={this.createTableAs.bind(this)}
tooltip={ctasToolTip}
>
<i className="fa fa-table" /> CTAS
</Button>
{this.props.database.allow_ctas && (
<Button
bsSize="small"
disabled={this.state.ctas.length === 0}
onClick={this.createTableAs.bind(this)}
tooltip={ctasToolTip}
>
<i className="fa fa-table" /> CTAS
</Button>
)}
{this.props.database.allow_cvas && (
<Button
bsSize="small"
disabled={this.state.ctas.length === 0}
onClick={this.createViewAs.bind(this)}
tooltip={cvasToolTip}
>
<i className="fa fa-table" /> CVAS
</Button>
)}
</InputGroup.Button>
</InputGroup>
</FormGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Add ctas_method to the Query object

Revision ID: ea396d202291
Revises: e557699a813e
Create Date: 2020-05-12 12:59:26.583276

"""

# revision identifiers, used by Alembic.
revision = "ea396d202291"
down_revision = "e557699a813e"

import sqlalchemy as sa
from alembic import op


def upgrade():
op.add_column(
"query", sa.Column("ctas_method", sa.String(length=16), nullable=True)
)
op.add_column("dbs", sa.Column("allow_cvas", sa.Boolean(), nullable=True))


def downgrade():
op.drop_column("query", "ctas_method")
op.drop_column("dbs", "allow_cvas")
2 changes: 2 additions & 0 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class Database(
allow_run_async = Column(Boolean, default=False)
allow_csv_upload = Column(Boolean, default=False)
allow_ctas = Column(Boolean, default=False)
allow_cvas = Column(Boolean, default=False)
allow_dml = Column(Boolean, default=False)
force_ctas_schema = Column(String(250))
allow_multi_schema_metadata_fetch = Column( # pylint: disable=invalid-name
Expand Down Expand Up @@ -147,6 +148,7 @@ class Database(
"expose_in_sqllab",
"allow_run_async",
"allow_ctas",
"allow_cvas",
"allow_csv_upload",
"extra",
]
Expand Down
3 changes: 2 additions & 1 deletion superset/models/sql_lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from datetime import datetime
from typing import Any, Dict

# pylint: disable=ungrouped-imports
import simplejson as json
import sqlalchemy as sqla
from flask import Markup
Expand All @@ -40,6 +39,7 @@
from superset import security_manager
from superset.models.helpers import AuditMixinNullable, ExtraJSONMixin
from superset.models.tags import QueryUpdater
from superset.sql_parse import CtasMethod
from superset.utils.core import QueryStatus, user_label


Expand Down Expand Up @@ -72,6 +72,7 @@ class Query(Model, ExtraJSONMixin):
limit = Column(Integer)
select_as_cta = Column(Boolean)
select_as_cta_used = Column(Boolean, default=False)
ctas_method = Column(String(16), default=CtasMethod.TABLE)

progress = Column(Integer, default=0) # 1..100
# # of rows in the result set or rows modified.
Expand Down
4 changes: 3 additions & 1 deletion superset/sql_lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ def execute_sql_statement(
query.user_id, start_dttm.strftime("%Y_%m_%d_%H_%M_%S")
)
sql = parsed_query.as_create_table(
query.tmp_table_name, schema_name=query.tmp_schema_name
query.tmp_table_name,
schema_name=query.tmp_schema_name,
method=query.ctas_method,
)
query.select_as_cta_used = True

Expand Down
12 changes: 10 additions & 2 deletions superset/sql_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.
import logging
from dataclasses import dataclass
from enum import Enum
from typing import List, Optional, Set
from urllib import parse

Expand All @@ -31,6 +32,11 @@
logger = logging.getLogger(__name__)


class CtasMethod(str, Enum):
TABLE = "TABLE"
VIEW = "VIEW"


def _extract_limit_from_query(statement: TokenList) -> Optional[int]:
"""
Extract limit clause from SQL statement.
Expand Down Expand Up @@ -185,6 +191,7 @@ def as_create_table(
table_name: str,
schema_name: Optional[str] = None,
overwrite: bool = False,
method: CtasMethod = CtasMethod.TABLE,
) -> str:
"""Reformats the query into the create table as query.

Expand All @@ -193,15 +200,16 @@ def as_create_table(
:param table_name: table that will contain the results of the query execution
:param schema_name: schema name for the target table
:param overwrite: table_name will be dropped if true
:param method: method for the CTA query, currently view or table creation
:return: Create table as query
"""
exec_sql = ""
sql = self.stripped()
# TODO(bkyryliuk): quote full_table_name
full_table_name = f"{schema_name}.{table_name}" if schema_name else table_name
if overwrite:
exec_sql = f"DROP TABLE IF EXISTS {full_table_name};\n"
exec_sql += f"CREATE TABLE {full_table_name} AS \n{sql}"
exec_sql = f"DROP {method} IF EXISTS {full_table_name};\n"
exec_sql += f"CREATE {method} {full_table_name} AS \n{sql}"
return exec_sql

def _extract_from_token( # pylint: disable=too-many-branches
Expand Down
7 changes: 6 additions & 1 deletion superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
check_sqlalchemy_uri,
DBSecurityException,
)
from superset.sql_parse import ParsedQuery, Table
from superset.sql_parse import CtasMethod, ParsedQuery, Table
from superset.sql_validators import get_validator_by_name
from superset.typing import FlaskResponse
from superset.utils import core as utils, dashboard_import_export
Expand Down Expand Up @@ -132,6 +132,7 @@
DATABASE_KEYS = [
"allow_csv_upload",
"allow_ctas",
"allow_cvas",
"allow_dml",
"allow_multi_schema_metadata_fetch",
"allow_run_async",
Expand Down Expand Up @@ -2240,6 +2241,9 @@ def sql_json_exec(
)
limit = 0
select_as_cta: bool = cast(bool, query_params.get("select_as_cta"))
ctas_method: CtasMethod = cast(
CtasMethod, query_params.get("ctas_method", CtasMethod.TABLE)
)
tmp_table_name: str = cast(str, query_params.get("tmp_table_name"))
client_id: str = cast(
str, query_params.get("client_id") or utils.shortid()[:10]
Expand Down Expand Up @@ -2268,6 +2272,7 @@ def sql_json_exec(
sql=sql,
schema=schema,
select_as_cta=select_as_cta,
ctas_method=ctas_method,
start_time=now_as_float(),
tab_name=tab_name,
status=status,
Expand Down
1 change: 1 addition & 0 deletions superset/views/database/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class DatabaseRestApi(DatabaseMixin, BaseSupersetModelRestApi):
"database_name",
"expose_in_sqllab",
"allow_ctas",
"allow_cvas",
"force_ctas_schema",
"allow_run_async",
"allow_dml",
Expand Down
3 changes: 3 additions & 0 deletions superset/views/database/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class DatabaseMixin:
"allow_run_async",
"allow_csv_upload",
"allow_ctas",
"allow_cvas",
"allow_dml",
"force_ctas_schema",
"impersonate_user",
Expand Down Expand Up @@ -111,6 +112,7 @@ class DatabaseMixin:
"for more information."
),
"allow_ctas": _("Allow CREATE TABLE AS option in SQL Lab"),
"allow_cvas": _("Allow CREATE VIEW AS option in SQL Lab"),
"allow_dml": _(
"Allow users to run non-SELECT statements "
"(UPDATE, DELETE, CREATE, ...) "
Expand Down Expand Up @@ -182,6 +184,7 @@ class DatabaseMixin:
label_columns = {
"expose_in_sqllab": _("Expose in SQL Lab"),
"allow_ctas": _("Allow CREATE TABLE AS"),
"allow_cvas": _("Allow CREATE VIEW AS"),
"allow_dml": _("Allow DML"),
"force_ctas_schema": _("CTAS Schema"),
"database_name": _("Database"),
Expand Down
3 changes: 3 additions & 0 deletions tests/base_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from flask_testing import TestCase
from sqlalchemy.orm import Session

from superset.sql_parse import CtasMethod
from tests.test_app import app # isort:skip
from superset import db, security_manager
from superset.connectors.base.models import BaseDatasource
Expand Down Expand Up @@ -259,6 +260,7 @@ def run_sql(
select_as_cta=False,
tmp_table_name=None,
schema=None,
ctas_method=CtasMethod.TABLE,
):
if user_name:
self.logout()
Expand All @@ -270,6 +272,7 @@ def run_sql(
"client_id": client_id,
"queryLimit": query_limit,
"sql_editor_id": sql_editor_id,
"ctas_method": ctas_method,
}
if tmp_table_name:
json_payload["tmp_table_name"] = tmp_table_name
Expand Down
Loading