From 156b0aaa0705e5b3900aa3b43d50a2fef01aee54 Mon Sep 17 00:00:00 2001 From: Alex Berghage Date: Wed, 15 May 2019 16:32:40 -0600 Subject: [PATCH 01/35] feat: Live query validation in the SQL Lab UI (#7461) (#7516) (#7518) * [WIP] Live query validation, where supported This builds on #7422 to build check-as-you-type sql query validation in Sql Lab. This closes #6707 too. It adds a (debounced) call to the validate_sql_json API endpoint with the querytext, and on Lyft infra is able to return feedback to the user (end to end) in $TBD seconds. At present feedback is provided only through the "annotations" mechanism build in to ACE, although I'd be open to adding full text elsewhere on the page if there's interest. * fix: Unbreak lints and tests --- superset/assets/src/SqlLab/actions/sqlLab.js | 54 +++++++++++++++ .../SqlLab/components/AceEditorWrapper.jsx | 15 +++++ .../src/SqlLab/components/SqlEditor.jsx | 34 ++++++++++ .../src/SqlLab/reducers/getInitialState.js | 5 ++ superset/assets/src/SqlLab/reducers/sqlLab.js | 65 +++++++++++++++++++ superset/assets/src/featureFlags.ts | 1 + superset/views/core.py | 5 +- 7 files changed, 176 insertions(+), 3 deletions(-) diff --git a/superset/assets/src/SqlLab/actions/sqlLab.js b/superset/assets/src/SqlLab/actions/sqlLab.js index f6ac455fff4f2..81c8e8d5593ec 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -65,6 +65,10 @@ export const CLEAR_QUERY_RESULTS = 'CLEAR_QUERY_RESULTS'; export const REMOVE_DATA_PREVIEW = 'REMOVE_DATA_PREVIEW'; export const CHANGE_DATA_PREVIEW_ID = 'CHANGE_DATA_PREVIEW_ID'; +export const START_QUERY_VALIDATION = 'START_QUERY_VALIDATION'; +export const QUERY_VALIDATION_RETURNED = 'QUERY_VALIDATION_RETURNED'; +export const QUERY_VALIDATION_FAILED = 'QUERY_VALIDATION_FAILED'; + export const CREATE_DATASOURCE_STARTED = 'CREATE_DATASOURCE_STARTED'; export const CREATE_DATASOURCE_SUCCESS = 'CREATE_DATASOURCE_SUCCESS'; export const CREATE_DATASOURCE_FAILED = 'CREATE_DATASOURCE_FAILED'; @@ -77,6 +81,21 @@ export function resetState() { return { type: RESET_STATE }; } +export function startQueryValidation(query) { + Object.assign(query, { + id: query.id ? query.id : shortid.generate(), + }); + return { type: START_QUERY_VALIDATION, query }; +} + +export function queryValidationReturned(query, results) { + return { type: QUERY_VALIDATION_RETURNED, query, results }; +} + +export function queryValidationFailed(query, message, error) { + return { type: QUERY_VALIDATION_FAILED, query, message, error }; +} + export function saveQuery(query) { return dispatch => SupersetClient.post({ @@ -187,6 +206,41 @@ export function runQuery(query) { }; } +export function validateQuery(query) { + return function (dispatch) { + dispatch(startQueryValidation(query)); + + const postPayload = { + client_id: query.id, + database_id: query.dbId, + json: true, + schema: query.schema, + sql: query.sql, + sql_editor_id: query.sqlEditorId, + templateParams: query.templateParams, + validate_only: true, + }; + + return SupersetClient.post({ + endpoint: `/superset/validate_sql_json/${window.location.search}`, + postPayload, + stringify: false, + }) + .then(({ json }) => { + dispatch(queryValidationReturned(query, json)); + }) + .catch(response => + getClientErrorObject(response).then((error) => { + let message = error.error || error.statusText || t('Unknown error'); + if (message.includes('CSRF token')) { + message = t(COMMON_ERR_MESSAGES.SESSION_TIMED_OUT); + } + dispatch(queryValidationFailed(query, message, error)); + }), + ); + }; +} + export function postStopQuery(query) { return function (dispatch) { return SupersetClient.post({ diff --git a/superset/assets/src/SqlLab/components/AceEditorWrapper.jsx b/superset/assets/src/SqlLab/components/AceEditorWrapper.jsx index 14a9775d902d1..6c87ec27c56c3 100644 --- a/superset/assets/src/SqlLab/components/AceEditorWrapper.jsx +++ b/superset/assets/src/SqlLab/components/AceEditorWrapper.jsx @@ -157,6 +157,20 @@ class AceEditorWrapper extends React.PureComponent { } }); } + getAceAnnotations() { + const validationResult = this.props.queryEditor.validationResult; + const resultIsReady = (validationResult && validationResult.completed); + if (resultIsReady && validationResult.errors.length > 0) { + const errors = validationResult.errors.map(err => ({ + type: 'error', + row: err.line_number - 1, + column: err.start_column - 1, + text: err.message, + })); + return errors; + } + return []; + } render() { return ( ); } diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx index f4495af2a4dc6..0b25dcf20ecad 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -30,6 +30,7 @@ import { } from 'react-bootstrap'; import Split from 'react-split'; import { t } from '@superset-ui/translation'; +import debounce from 'lodash/debounce'; import Button from '../../components/Button'; import LimitControl from './LimitControl'; @@ -52,6 +53,7 @@ const GUTTER_HEIGHT = 5; const GUTTER_MARGIN = 3; const INITIAL_NORTH_PERCENT = 30; const INITIAL_SOUTH_PERCENT = 70; +const VALIDATION_DEBOUNCE_MS = 600; const propTypes = { actions: PropTypes.object.isRequired, @@ -88,6 +90,7 @@ class SqlEditor extends React.PureComponent { this.elementStyle = this.elementStyle.bind(this); this.onResizeStart = this.onResizeStart.bind(this); this.onResizeEnd = this.onResizeEnd.bind(this); + this.canValidateQuery = this.canValidateQuery.bind(this); this.runQuery = this.runQuery.bind(this); this.stopQuery = this.stopQuery.bind(this); this.onSqlChanged = this.onSqlChanged.bind(this); @@ -95,6 +98,10 @@ class SqlEditor extends React.PureComponent { this.queryPane = this.queryPane.bind(this); this.getAceEditorAndSouthPaneHeights = this.getAceEditorAndSouthPaneHeights.bind(this); this.getSqlEditorHeight = this.getSqlEditorHeight.bind(this); + this.requestValidation = debounce( + this.requestValidation.bind(this), + VALIDATION_DEBOUNCE_MS, + ); } componentWillMount() { if (this.state.autorun) { @@ -126,6 +133,11 @@ class SqlEditor extends React.PureComponent { } onSqlChanged(sql) { this.setState({ sql }); + // Request server-side validation of the query text + if (this.canValidateQuery()) { + // NB. requestValidation is debounced + this.requestValidation(); + } } // One layer of abstraction for easy spying in unit tests getSqlEditorHeight() { @@ -186,6 +198,28 @@ class SqlEditor extends React.PureComponent { [dimension]: `calc(${elementSize}% - ${gutterSize + GUTTER_MARGIN}px)`, }; } + requestValidation() { + if (this.props.database) { + const qe = this.props.queryEditor; + const query = { + dbId: qe.dbId, + sql: this.state.sql, + sqlEditorId: qe.id, + schema: qe.schema, + templateParams: qe.templateParams, + }; + this.props.actions.validateQuery(query); + } + } + canValidateQuery() { + // Check whether or not we can validate the current query based on whether + // or not the backend has a validator configured for it. + const validatorMap = window.featureFlags.SQL_VALIDATORS_BY_ENGINE; + if (this.props.database && validatorMap != null) { + return validatorMap.hasOwnProperty(this.props.database.backend); + } + return false; + } runQuery() { if (this.props.database) { this.startQuery(this.props.database.allow_run_async); diff --git a/superset/assets/src/SqlLab/reducers/getInitialState.js b/superset/assets/src/SqlLab/reducers/getInitialState.js index adb3db43f6606..535cb3dc77de3 100644 --- a/superset/assets/src/SqlLab/reducers/getInitialState.js +++ b/superset/assets/src/SqlLab/reducers/getInitialState.js @@ -30,6 +30,11 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) { autorun: false, dbId: defaultDbId, queryLimit: restBootstrapData.common.conf.DEFAULT_SQLLAB_LIMIT, + validationResult: { + id: null, + errors: [], + completed: false, + }, }; return { diff --git a/superset/assets/src/SqlLab/reducers/sqlLab.js b/superset/assets/src/SqlLab/reducers/sqlLab.js index 93ff32556bfdc..a94e817616245 100644 --- a/superset/assets/src/SqlLab/reducers/sqlLab.js +++ b/superset/assets/src/SqlLab/reducers/sqlLab.js @@ -141,6 +141,71 @@ export default function sqlLabReducer(state = {}, action) { [actions.REMOVE_TABLE]() { return removeFromArr(state, 'tables', action.table); }, + [actions.START_QUERY_VALIDATION]() { + let newState = Object.assign({}, state); + const sqlEditor = { id: action.query.sqlEditorId }; + newState = alterInArr(newState, 'queryEditors', sqlEditor, { + validationResult: { + id: action.query.id, + errors: [], + completed: false, + }, + }); + return newState; + }, + [actions.QUERY_VALIDATION_RETURNED]() { + // If the server is very slow about answering us, we might get validation + // responses back out of order. This check confirms the response we're + // handling corresponds to the most recently dispatched request. + // + // We don't care about any but the most recent because validations are + // only valid for the SQL text they correspond to -- once the SQL has + // changed, the old validation doesn't tell us anything useful anymore. + const qe = getFromArr(state.queryEditors, action.query.sqlEditorId); + if (qe.validationResult.id !== action.query.id) { + return state; + } + // Otherwise, persist the results on the queryEditor state + let newState = Object.assign({}, state); + const sqlEditor = { id: action.query.sqlEditorId }; + newState = alterInArr(newState, 'queryEditors', sqlEditor, { + validationResult: { + id: action.query.id, + errors: action.results, + completed: true, + }, + }); + return newState; + }, + [actions.QUERY_VALIDATION_FAILED]() { + // If the server is very slow about answering us, we might get validation + // responses back out of order. This check confirms the response we're + // handling corresponds to the most recently dispatched request. + // + // We don't care about any but the most recent because validations are + // only valid for the SQL text they correspond to -- once the SQL has + // changed, the old validation doesn't tell us anything useful anymore. + const qe = getFromArr(state.queryEditors, action.query.sqlEditorId); + if (qe.validationResult.id !== action.query.id) { + return state; + } + // Otherwise, persist the results on the queryEditor state + let newState = Object.assign({}, state); + const sqlEditor = { id: action.query.sqlEditorId }; + newState = alterInArr(newState, 'queryEditors', sqlEditor, { + validationResult: { + id: action.query.id, + errors: [{ + line_number: 1, + start_column: 1, + end_column: 1, + message: `The server failed to validate your query.\n${action.message}`, + }], + completed: true, + }, + }); + return newState; + }, [actions.START_QUERY]() { let newState = Object.assign({}, state); if (action.query.sqlEditorId) { diff --git a/superset/assets/src/featureFlags.ts b/superset/assets/src/featureFlags.ts index 450ad2cd4f896..bd0855ef18461 100644 --- a/superset/assets/src/featureFlags.ts +++ b/superset/assets/src/featureFlags.ts @@ -23,6 +23,7 @@ export enum FeatureFlag { OMNIBAR = 'OMNIBAR', CLIENT_CACHE = 'CLIENT_CACHE', SCHEDULED_QUERIES = 'SCHEDULED_QUERIES', + SQL_VALIDATORS_BY_ENGINE = 'SQL_VALIDATORS_BY_ENGINE', } export type FeatureFlagMap = { diff --git a/superset/views/core.py b/superset/views/core.py index 468ea58e4e82f..e79de112b8dab 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2579,9 +2579,8 @@ def validate_sql_json(self): except Exception as e: logging.exception(e) msg = _( - 'Failed to validate your SQL query text. Please check that ' - f'you have configured the {validator.name} validator ' - 'correctly and that any services it depends on are up. ' + f'{validator.name} was unable to check your query.\nPlease ' + 'make sure that any services it depends on are available\n' f'Exception: {e}') return json_error_response(f'{msg}') From 9423e9a8be1c0fc0bad8fbd2f04fe298d08469bf Mon Sep 17 00:00:00 2001 From: Alex Berghage Date: Wed, 15 May 2019 16:33:30 -0600 Subject: [PATCH 02/35] chore: Truncate progressbar percentage decimals (#7499) (#7517) (#7519) This change makes the query progress bar only show whole number percentage changes, instead of numbers like 12.13168276%. --- superset/assets/src/SqlLab/components/QueryTable.jsx | 6 +++--- superset/assets/src/SqlLab/components/ResultSet.jsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/superset/assets/src/SqlLab/components/QueryTable.jsx b/superset/assets/src/SqlLab/components/QueryTable.jsx index 92045bd2ee277..5cf122d03652e 100644 --- a/superset/assets/src/SqlLab/components/QueryTable.jsx +++ b/superset/assets/src/SqlLab/components/QueryTable.jsx @@ -41,8 +41,8 @@ const propTypes = { const defaultProps = { columns: ['started', 'duration', 'rows'], queries: [], - onUserClicked: () => {}, - onDbClicked: () => {}, + onUserClicked: () => { }, + onDbClicked: () => { }, }; class QueryTable extends React.PureComponent { @@ -169,7 +169,7 @@ class QueryTable extends React.PureComponent { style={{ width: '75px' }} striped now={q.progress} - label={`${q.progress}%`} + label={`${q.progress.toFixed(0)}%`} /> ); let errorTooltip; diff --git a/superset/assets/src/SqlLab/components/ResultSet.jsx b/superset/assets/src/SqlLab/components/ResultSet.jsx index 22b6b0d46d55f..443eb16b29998 100644 --- a/superset/assets/src/SqlLab/components/ResultSet.jsx +++ b/superset/assets/src/SqlLab/components/ResultSet.jsx @@ -241,7 +241,7 @@ export default class ResultSet extends React.PureComponent { ); } if (query.trackingUrl) { From 7f858e4566bd4944a904c0530709e0ba0a89eb5e Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Thu, 16 May 2019 15:22:23 -0700 Subject: [PATCH 03/35] [sql lab] Fix new query stuck at pending state (#7523) --- superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx index 8ad02adad3bf9..21d335b03edcc 100644 --- a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx +++ b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx @@ -38,7 +38,7 @@ class QueryAutoRefresh extends React.PureComponent { } shouldCheckForQueries() { // if there are started or running queries, this method should return true - const { queries, queriesLastUpdate } = this.props; + const { queries } = this.props; const now = new Date().getTime(); // due to a race condition, queries can be marked as successful before the @@ -50,7 +50,6 @@ class QueryAutoRefresh extends React.PureComponent { ); return ( - queriesLastUpdate > 0 && Object.values(queries).some( q => isQueryRunning(q) && now - q.startDttm < MAX_QUERY_AGE_TO_POLL, From 21a467094ba3e194b9799026f0706c5453a5fb1e Mon Sep 17 00:00:00 2001 From: Craig Rueda Date: Thu, 16 May 2019 20:55:59 -0700 Subject: [PATCH 04/35] Talisman config (#7529) * Making Talisman configurable * Fixing double quotes * Fixing flake8 * Removing default --- superset/__init__.py | 4 +++- superset/config.py | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/superset/__init__.py b/superset/__init__.py index 47811dbc2566a..6971dc9d4c8ab 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -230,7 +230,9 @@ def is_feature_enabled(feature): if conf.get('ENABLE_FLASK_COMPRESS'): Compress(app) -Talisman(app, content_security_policy=None) +if app.config['TALISMAN_ENABLED']: + talisman_config = app.config.get('TALISMAN_CONFIG') + Talisman(app, **talisman_config) # Hook that provides administrators a handle on the Flask APP # after initialization diff --git a/superset/config.py b/superset/config.py index d191a28c793a3..4949bf903b45f 100644 --- a/superset/config.py +++ b/superset/config.py @@ -612,6 +612,15 @@ class CeleryConfig(object): 'presto': 'PrestoDBSQLValidator', } +# Do you want Talisman enabled? +TALISMAN_ENABLED = True +# If you want Talisman, how do you want it configured?? +TALISMAN_CONFIG = { + 'content_security_policy': None, + 'force_https': True, + 'force_https_permanent': False, +} + try: if CONFIG_PATH_ENV_VAR in os.environ: # Explicitly import config module that is not in pythonpath; useful From f0f719cb8ddf341b240c2af6db90392b8d23e703 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 17 May 2019 17:30:13 -0700 Subject: [PATCH 05/35] Validate start/end when scheduling queries (#7544) * Validate start/end when scheduling queries * Use chrono instead of Sugar --- docs/installation.rst | 20 +++++++- superset/assets/package-lock.json | 15 ++++++ superset/assets/package.json | 1 + .../SqlLab/components/ScheduleQueryButton.jsx | 50 ++++++++++++++++++- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index c7c24fc1d4e67..82459c7c53eea 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -858,13 +858,19 @@ To allow scheduled queries, add the following to your `config.py`: }, 'start_date': { 'type': 'string', - 'format': 'date-time', 'title': 'Start date', + # date-time is parsed using the chrono library, see + # https://www.npmjs.com/package/chrono-node#usage + 'format': 'date-time', + 'default': 'tomorrow at 9am', }, 'end_date': { 'type': 'string', - 'format': 'date-time', 'title': 'End date', + # date-time is parsed using the chrono library, see + # https://www.npmjs.com/package/chrono-node#usage + 'format': 'date-time', + 'default': '9am in 30 days', }, 'schedule_interval': { 'type': 'string', @@ -890,6 +896,16 @@ To allow scheduled queries, add the following to your `config.py`: ), }, }, + 'VALIDATION': [ + # ensure that start_date <= end_date + { + 'name': 'less_equal', + 'arguments': ['start_date', 'end_date'], + 'message': 'End date cannot be before start date', + # this is where the error message is shown + 'container': 'end_date', + }, + ], }, } diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json index e5075d9699665..da886a0c95d17 100644 --- a/superset/assets/package-lock.json +++ b/superset/assets/package-lock.json @@ -5662,6 +5662,21 @@ "tslib": "^1.9.0" } }, + "chrono-node": { + "version": "1.3.11", + "resolved": "https://registry.npmjs.org/chrono-node/-/chrono-node-1.3.11.tgz", + "integrity": "sha512-jDWRnY6nYvzfV3HPYBqo+tot7tcsUs9i3arGbMdI0TouPSXP2C2y/Ctp27rxKTQDi6yuTxAB2cw+Q6igGhOhdQ==", + "requires": { + "moment": "2.21.0" + }, + "dependencies": { + "moment": { + "version": "2.21.0", + "resolved": "https://registry.npmjs.org/moment/-/moment-2.21.0.tgz", + "integrity": "sha512-TCZ36BjURTeFTM/CwRcViQlfkMvL1/vFISuNLO5GkcVm1+QHfbSiNqZuWeMFjj1/3+uAjXswgRk30j1kkLYJBQ==" + } + } + }, "ci-info": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/ci-info/-/ci-info-2.0.0.tgz", diff --git a/superset/assets/package.json b/superset/assets/package.json index 6edb971331dc9..52e82b12e01a1 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -84,6 +84,7 @@ "bootstrap": "^3.3.6", "bootstrap-slider": "^10.0.0", "brace": "^0.11.1", + "chrono-node": "^1.3.11", "classnames": "^2.2.5", "d3-array": "^1.2.4", "d3-color": "^1.2.0", diff --git a/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx b/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx index 2e7e16e3167af..99ecaa5256052 100644 --- a/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx +++ b/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx @@ -19,11 +19,56 @@ import React from 'react'; import PropTypes from 'prop-types'; import Form from 'react-jsonschema-form'; +import chrono from 'chrono-node'; import { t } from '@superset-ui/translation'; import Button from '../../components/Button'; import ModalTrigger from '../../components/ModalTrigger'; +const validators = { + greater: (a, b) => a > b, + greater_equal: (a, b) => a >= b, + less: (a, b) => a < b, + less_equal: (a, b) => a <= b, +}; + +function getJSONSchema() { + const jsonSchema = window.featureFlags.SCHEDULED_QUERIES.JSONSCHEMA; + // parse date-time into usable value (eg, 'today' => `new Date()`) + Object.entries(jsonSchema.properties).forEach(([key, properties]) => { + if (properties.default && properties.format === 'date-time') { + jsonSchema.properties[key] = { + ...properties, + default: chrono.parseDate(properties.default).toISOString(), + }; + } + }); + return jsonSchema; +} + +function getUISchema() { + return window.featureFlags.SCHEDULED_QUERIES.UISCHEMA; +} + +function getValidationRules() { + return window.featureFlags.SCHEDULED_QUERIES.VALIDATION || []; +} + +function getValidator() { + const rules = getValidationRules(); + return (formData, errors) => { + rules.forEach((rule) => { + const test = validators[rule.name]; + const args = rule.arguments.map(name => formData[name]); + const container = rule.container || rule.arguments.slice(-1)[0]; + if (!test(...args)) { + errors[container].addError(rule.message); + } + }); + return errors; + }; +} + const propTypes = { defaultLabel: PropTypes.string, sql: PropTypes.string.isRequired, @@ -79,9 +124,10 @@ class ScheduleQueryButton extends React.PureComponent { renderModalBody() { return (
); } From dcafabd1833b92cb26c265ff5a0297a7b226b025 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 17 May 2019 17:31:02 -0700 Subject: [PATCH 06/35] Show scheduled queries (#7545) * Show scheduled queries * Remove column * Secure views * Add import * Fix unit tests * Reuse existing db connection from view * Remove unnecessary import --- superset/assets/src/showSavedQuery/index.jsx | 43 +++++++++++++++++++ superset/assets/webpack.config.js | 1 + .../superset/models/savedquery/show.html | 35 +++++++++++++++ superset/views/base.py | 2 +- superset/views/core.py | 10 ++--- superset/views/sql_lab.py | 35 ++++++++++++++- 6 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 superset/assets/src/showSavedQuery/index.jsx create mode 100644 superset/templates/superset/models/savedquery/show.html diff --git a/superset/assets/src/showSavedQuery/index.jsx b/superset/assets/src/showSavedQuery/index.jsx new file mode 100644 index 0000000000000..cd384fe5b461e --- /dev/null +++ b/superset/assets/src/showSavedQuery/index.jsx @@ -0,0 +1,43 @@ +/** + * 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. + */ +import React from 'react'; +import ReactDom from 'react-dom'; +import Form from 'react-jsonschema-form'; + +const scheduleInfoContainer = document.getElementById('schedule-info'); +const bootstrapData = JSON.parse(scheduleInfoContainer.getAttribute('data-bootstrap')); +const schemas = bootstrapData.common.feature_flags.SCHEDULED_QUERIES; +const scheduleInfo = bootstrapData.common.extra_json.schedule_info; + +if (scheduleInfo && schemas) { + // hide instructions when showing schedule info + schemas.JSONSCHEMA.description = ''; + + ReactDom.render( + +
+ , + scheduleInfoContainer, + ); +} diff --git a/superset/assets/webpack.config.js b/superset/assets/webpack.config.js index 97a51de8130e5..241c5b962f162 100644 --- a/superset/assets/webpack.config.js +++ b/superset/assets/webpack.config.js @@ -119,6 +119,7 @@ const config = { sqllab: addPreamble('/src/SqlLab/index.jsx'), welcome: addPreamble('/src/welcome/index.jsx'), profile: addPreamble('/src/profile/index.jsx'), + showSavedQuery: [path.join(APP_DIR, '/src/showSavedQuery/index.jsx')], }, output, optimization: { diff --git a/superset/templates/superset/models/savedquery/show.html b/superset/templates/superset/models/savedquery/show.html new file mode 100644 index 0000000000000..9ffedeedbfac5 --- /dev/null +++ b/superset/templates/superset/models/savedquery/show.html @@ -0,0 +1,35 @@ +{# + 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. +#} +{% extends "appbuilder/general/model/show.html" %} + +{% block show_form %} + {{ super() }} +
+{% endblock %} + +{% block tail_js %} + {{ super() }} + {% with filename="showSavedQuery" %} + {% include "superset/partials/_script_tag.html" %} + {% endwith %} +{% endblock %} diff --git a/superset/views/base.py b/superset/views/base.py index 113b9c68e3975..2d14622ea893c 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -153,7 +153,7 @@ def json_response(self, obj, status=200): status=status, mimetype='application/json') - def common_bootsrap_payload(self): + def common_bootstrap_payload(self): """Common data always sent to the client""" messages = get_flashed_messages(with_categories=True) locale = str(get_locale()) diff --git a/superset/views/core.py b/superset/views/core.py index e79de112b8dab..1f69f4049f7ed 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1372,7 +1372,7 @@ def explore(self, datasource_type=None, datasource_id=None): 'standalone': standalone, 'user_id': user_id, 'forced_height': request.args.get('height'), - 'common': self.common_bootsrap_payload(), + 'common': self.common_bootstrap_payload(), } table_name = datasource.table_name \ if datasource_type == 'table' \ @@ -2231,7 +2231,7 @@ def dashboard(**kwargs): # noqa 'user_id': g.user.get_id(), 'dashboard_data': dashboard_data, 'datasources': {ds.uid: ds.data for ds in datasources}, - 'common': self.common_bootsrap_payload(), + 'common': self.common_bootstrap_payload(), 'editMode': edit_mode, } @@ -2919,7 +2919,7 @@ def welcome(self): payload = { 'user': bootstrap_user_data(), - 'common': self.common_bootsrap_payload(), + 'common': self.common_bootstrap_payload(), } return self.render_template( @@ -2938,7 +2938,7 @@ def profile(self, username): payload = { 'user': bootstrap_user_data(username, include_perms=True), - 'common': self.common_bootsrap_payload(), + 'common': self.common_bootstrap_payload(), } return self.render_template( @@ -2954,7 +2954,7 @@ def sqllab(self): """SQL Editor""" d = { 'defaultDbId': config.get('SQLLAB_DEFAULT_DBID'), - 'common': self.common_bootsrap_payload(), + 'common': self.common_bootstrap_payload(), } return self.render_template( 'superset/basic.html', diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py index b9d1f2eb35097..37c84c2705253 100644 --- a/superset/views/sql_lab.py +++ b/superset/views/sql_lab.py @@ -20,13 +20,15 @@ from flask import g, redirect from flask_appbuilder import expose from flask_appbuilder.models.sqla.interface import SQLAInterface -from flask_appbuilder.security.decorators import has_access +from flask_appbuilder.security.decorators import has_access, has_access_api from flask_babel import gettext as __ from flask_babel import lazy_gettext as _ from flask_sqlalchemy import BaseQuery +import simplejson as json -from superset import appbuilder, security_manager +from superset import appbuilder, get_feature_flags, security_manager from superset.models.sql_lab import Query, SavedQuery +from superset.utils import core as utils from .base import BaseSupersetView, DeleteMixin, SupersetFilter, SupersetModelView @@ -107,12 +109,36 @@ class SavedQueryView(SupersetModelView, DeleteMixin): 'changed_on': _('Changed on'), } + show_template = 'superset/models/savedquery/show.html' + def pre_add(self, obj): obj.user = g.user def pre_update(self, obj): self.pre_add(obj) + @has_access + @expose('show/') + def show(self, pk): + pk = self._deserialize_pk_if_composite(pk) + widgets = self._show(pk) + extra_json = self.datamodel.get(pk).extra_json + payload = { + 'common': { + 'feature_flags': get_feature_flags(), + 'extra_json': json.loads(extra_json), + }, + } + + return self.render_template( + self.show_template, + pk=pk, + title=self.show_title, + widgets=widgets, + related_views=self._related_views, + bootstrap_data=json.dumps(payload, default=utils.json_iso_dttm_ser), + ) + class SavedQueryViewApi(SavedQueryView): list_columns = [ @@ -123,6 +149,11 @@ class SavedQueryViewApi(SavedQueryView): add_columns = show_columns edit_columns = add_columns + @has_access_api + @expose('show/') + def show(self, pk): + return super().show(pk) + appbuilder.add_view_no_menu(SavedQueryViewApi) appbuilder.add_view_no_menu(SavedQueryView) From c79077d85d844d5c4c5d2ff648b43593aa32b6ae Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Mon, 20 May 2019 12:04:42 -0700 Subject: [PATCH 07/35] feat: add header tooltip (#7556) * feat: add header tooltip (#7531) --- .../FilterableTable/FilterableTable.jsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/components/FilterableTable/FilterableTable.jsx b/superset/assets/src/components/FilterableTable/FilterableTable.jsx index 3ef03ba8ae9a0..702f2b96493f8 100644 --- a/superset/assets/src/components/FilterableTable/FilterableTable.jsx +++ b/superset/assets/src/components/FilterableTable/FilterableTable.jsx @@ -27,6 +27,7 @@ import { SortIndicator, } from 'react-virtualized'; import { getTextDimension } from '@superset-ui/dimension'; +import TooltipWrapper from '../TooltipWrapper'; function getTextWidth(text, font = '12px Roboto') { return getTextDimension({ text, style: { font } }).width; @@ -138,12 +139,14 @@ export default class FilterableTable extends PureComponent { headerRenderer({ dataKey, label, sortBy, sortDirection }) { return ( -
- {label} - {sortBy === dataKey && - - } -
+ +
+ {label} + {sortBy === dataKey && + + } +
+
); } From 1fdc96a3817e802209b0ffe580ff6f12c7335551 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Mon, 20 May 2019 16:58:36 -0700 Subject: [PATCH 08/35] Disabling flask-talisman by default (#7535) flask-talisman was enabled recently and while it may be virtuous in some cases, it seems to break things out of the box. Locally and in dev mode, upon my first redirect it sends to HTTPS and things it crashes. I think it should be opt-in, maybe we can recommend turning this on in production in the docs? --- superset/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/config.py b/superset/config.py index 4949bf903b45f..5f9bd8a3bf2f9 100644 --- a/superset/config.py +++ b/superset/config.py @@ -613,7 +613,7 @@ class CeleryConfig(object): } # Do you want Talisman enabled? -TALISMAN_ENABLED = True +TALISMAN_ENABLED = False # If you want Talisman, how do you want it configured?? TALISMAN_CONFIG = { 'content_security_policy': None, From 023faf3b5655b9b48813efdaab116076fb96b067 Mon Sep 17 00:00:00 2001 From: Russell Jurney Date: Mon, 20 May 2019 17:06:08 -0700 Subject: [PATCH 09/35] Rjurney master docs update (#7426) * resolved conflict * Docs updated re: Anaconda/certifi issue re #7373 * Removed --console-log "not working" note * A note about Anaconda virtualenvs * Make anaconda comment fit on page * Added README to docker directory * Added install doc reference to master copy of contrib/docker/README.md * merged master, removed mysqlclient * Removed mysql dependency, Anaconda and --console-log references * Add cypress install command to cypress test instructions * Fixed cypress instructions re: port 8081 * Removed anaconda reference, runserver references * Remove anaconda reference * Added back a self-contained version of mysqlclient to dev requirements * Added ASF license to docker README.md --- CONTRIBUTING.md | 42 +++++++++++++++++++++++------ contrib/docker/README.md | 58 ++++++++++++++++++++++++++++++++++++++++ docs/installation.rst | 19 ++++++++----- requirements-dev.txt | 2 +- 4 files changed, 105 insertions(+), 16 deletions(-) create mode 100644 contrib/docker/README.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e620e6afe8b4f..680f4f198ccc9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -294,7 +294,17 @@ python setup.py build_sphinx ### Flask server -Make sure your machine meets the [OS dependencies](https://superset.incubator.apache.org/installation.html#os-dependencies) before following these steps. +#### OS Dependencies + +Make sure your machine meets the [OS dependencies](https://superset.incubator.apache.org/installation.html#os-dependencies) before following these steps. + +Developers should use a virtualenv. + +``` +pip install virtualenv +``` + +Then proceed with: ```bash # Create a virtual environemnt and activate it (recommended) @@ -304,10 +314,11 @@ source venv/bin/activate # Install external dependencies pip install -r requirements.txt pip install -r requirements-dev.txt + # Install Superset in editable (development) mode pip install -e . -# Create an admin user +# Create an admin user in your metadata database fabmanager create-admin --app superset # Initialize the database @@ -319,11 +330,10 @@ superset init # Load some data to play with superset load_examples -# Start the Flask dev web server from inside the `superset` dir at port 8088 +# Start the Flask dev web server from inside your virtualenv. # Note that your page may not have css at this point. # See instructions below how to build the front-end assets. -cd superset -FLASK_ENV=development flask run -p 8088 --with-threads --reload --debugger +FLASK_ENV=development superset run -p 8088 --with-threads --reload --debugger ``` #### Logging to the browser console @@ -355,7 +365,14 @@ app.logger.info(form_data) Frontend assets (JavaScript, CSS, and images) must be compiled in order to properly display the web UI. The `superset/assets` directory contains all NPM-managed front end assets. Note that there are additional frontend assets bundled with Flask-Appbuilder (e.g. jQuery and bootstrap); these are not managed by NPM, and may be phased out in the future. -First, be sure you are using recent versions of NodeJS and npm. Using [nvm](https://github.com/creationix/nvm) to manage them is recommended. +#### nvm and node + +First, be sure you are using recent versions of NodeJS and npm. Using [nvm](https://github.com/creationix/nvm) to manage them is recommended. Check the docs at the link to be sure, but at the time of writing the following would install nvm and node: + +```bash +curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.34.0/install.sh | bash +nvm install node +``` #### Prerequisite @@ -396,6 +413,12 @@ npm run dev npm run prod ``` +If you run this service from somewhere other than your local machine, you may need to add hostname value to webpack.config.js at .devServer.public specifying the endpoint at which you will access the app. For example: myhost:9001. For convenience you may want to install webpack, webpack-cli and webpack-dev-server globally so that you can run them directly: + +```bash +npm install --global webpack webpack-cli webpack-dev-server +``` + #### Updating NPM packages Use npm in the prescribed way, making sure that @@ -517,14 +540,15 @@ superset db upgrade superset init superset load_test_users superset load_examples -superset runserver +superset run --port 8081 ``` Run Cypress tests: ```bash -cd /superset/superset/assets +cd superset/assets npm run build +npm run install-cypress npm run cypress run # run tests from a specific file @@ -534,6 +558,8 @@ npm run cypress run -- --spec cypress/integration/explore/link.test.js npm run cypress run -- --spec cypress/integration/dashboard/index.test.js --config video=true ``` +See [`superset/assets/cypress_build.sh`](https://github.com/apache/incubator-superset/blob/master/superset/assets/cypress_build.sh). + ## Translating We use [Babel](http://babel.pocoo.org/en/latest/) to translate Superset. In Python files, we import the magic `_` function using: diff --git a/contrib/docker/README.md b/contrib/docker/README.md new file mode 100644 index 0000000000000..0c3146f7e7347 --- /dev/null +++ b/contrib/docker/README.md @@ -0,0 +1,58 @@ + + +# Getting Start with Superset using Docker + +Docker is an easy way to get stated with Superset. + +## Initializing Database + +To initialize the database with a user and example charts, dashboards and datasets run: + +```bash +SUPERSET_LOAD_EXAMPLES=yes docker-compose run --rm superset ./docker-init.sh +``` + +This may take a minute. + +## Normal Operation + +To run the container, simply run: + +```bash +docker-compose up +``` + +After several minutes for superset initialization to finish, you can open a browser and view [`http://localhost:8088`](http://localhost:8088) +to start your journey. + +## Developing + +While running, the container server will reload on modification of the superset python and javascript source code. +Don't forget to reload the page to take the new frontend into account though. + +## Production + +It is also possible to run Superset in non-development mode: in the `docker-compose.yml` file remove +the volumes needed for development and change the variable `SUPERSET_ENV` to `production`. + +## Resource Constraints + +If you are attempting to build on a Mac and it exits with 137 you need to increase your docker resources. +OSX instructions: https://docs.docker.com/docker-for-mac/#advanced (Search for memory) diff --git a/docs/installation.rst b/docs/installation.rst index 82459c7c53eea..329debc8a0469 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -91,6 +91,8 @@ OSX instructions: https://docs.docker.com/docker-for-mac/#advanced (Search for m Or if you're curious and want to install superset from bottom up, then go ahead. +See also `contrib/docker/README.md `_ + OS dependencies --------------- @@ -121,7 +123,13 @@ that the required dependencies are installed: :: sudo yum upgrade python-setuptools sudo yum install gcc gcc-c++ libffi-devel python-devel python-pip python-wheel openssl-devel libsasl2-devel openldap-devel -**OSX**, system python is not recommended. brew's python also ships with pip :: +**Mac OS X** If possible, you should upgrade to the latest version of OS X as issues are more likely to be resolved for that version. +You *will likely need* the latest version of XCode available for your installed version of OS X. You should also install +the XCode command line tools: :: + + xcode-select --install + +System python is not recommended. Homebrew's python also ships with pip: :: brew install pkg-config libffi openssl python env LDFLAGS="-L$(brew --prefix openssl)/lib" CFLAGS="-I$(brew --prefix openssl)/include" pip install cryptography==2.4.2 @@ -184,8 +192,7 @@ Follow these few simple steps to install Superset.:: superset init # To start a development web server on port 8088, use -p to bind to another port - flask run -p 8080 --with-threads --reload --debugger - + superset run -p 8080 --with-threads --reload --debugger After installation, you should be able to point your browser to the right hostname:port `http://localhost:8088 `_, login using @@ -219,10 +226,8 @@ Refer to the `Gunicorn documentation `_ for more information. -Note that *gunicorn* does not -work on Windows so the `superset runserver` command is not expected to work -in that context. Also, note that the development web -server (`superset runserver -d`) is not intended for production use. +Note that the development web +server (`superset run` or `flask run`) is not intended for production use. If not using gunicorn, you may want to disable the use of flask-compress by setting `ENABLE_FLASK_COMPRESS = False` in your `superset_config.py` diff --git a/requirements-dev.txt b/requirements-dev.txt index 857b9ad07d09f..4b08ec4e43a54 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -23,7 +23,7 @@ flake8==3.6.0 flask-cors==3.0.6 ipdb==0.11 mypy==0.670 -mysqlclient==1.3.13 +mysqlclient==1.4.2.post1 nose==1.3.7 pip-tools==3.5.0 psycopg2-binary==2.7.5 From 74704f68c79f68de642732c7ece1e2682a5695bb Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Tue, 21 May 2019 01:30:00 +0100 Subject: [PATCH 10/35] [security] New, deprecate merge_perm, FAB method is fixed (#7355) * [security] New, deprecate merge_perm, FAB method is fixed * [style] Fix, flakes * [tests] Fix, change merge_perm to add_permission_view_menu * [security] Fix, maintain merge_perm for compatibility * [security] New, deprecation warning on merge_perm method * [style] Fix, flake8 C812 --- superset/cli.py | 2 +- superset/connectors/druid/views.py | 16 +++++++++++----- superset/connectors/sqla/views.py | 8 ++++---- superset/security.py | 26 +++++++++++--------------- superset/views/core.py | 4 ++-- tests/access_tests.py | 4 ++-- tests/druid_tests.py | 4 ++-- 7 files changed, 33 insertions(+), 31 deletions(-) diff --git a/superset/cli.py b/superset/cli.py index 52429104fbf79..7b441b47bdf9e 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -371,7 +371,7 @@ def load_test_users_run(): security_manager.add_permission_role(gamma_sqllab_role, perm) utils.get_or_create_main_db() db_perm = utils.get_main_database(security_manager.get_session).perm - security_manager.merge_perm('database_access', db_perm) + security_manager.add_permission_view_menu('database_access', db_perm) db_pvm = security_manager.find_permission_view_menu( view_menu_name=db_perm, permission_name='database_access') gamma_sqllab_role.permissions.append(db_pvm) diff --git a/superset/connectors/druid/views.py b/superset/connectors/druid/views.py index 4b05d70c7b4e3..99923e03162db 100644 --- a/superset/connectors/druid/views.py +++ b/superset/connectors/druid/views.py @@ -147,11 +147,11 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa def post_add(self, metric): if metric.is_restricted: - security_manager.merge_perm('metric_access', metric.get_perm()) + security_manager.add_permission_view_menu('metric_access', metric.get_perm()) def post_update(self, metric): if metric.is_restricted: - security_manager.merge_perm('metric_access', metric.get_perm()) + security_manager.add_permission_view_menu('metric_access', metric.get_perm()) appbuilder.add_view_no_menu(DruidMetricInlineView) @@ -202,7 +202,7 @@ class DruidClusterModelView(SupersetModelView, DeleteMixin, YamlExportMixin): # } def pre_add(self, cluster): - security_manager.merge_perm('database_access', cluster.perm) + security_manager.add_permission_view_menu('database_access', cluster.perm) def pre_update(self, cluster): self.pre_add(cluster) @@ -311,9 +311,15 @@ def pre_add(self, datasource): def post_add(self, datasource): datasource.refresh_metrics() - security_manager.merge_perm('datasource_access', datasource.get_perm()) + security_manager.add_permission_view_menu( + 'datasource_access', + datasource.get_perm(), + ) if datasource.schema: - security_manager.merge_perm('schema_access', datasource.schema_perm) + security_manager.add_permission_view_menu( + 'schema_access', + datasource.schema_perm, + ) def post_update(self, datasource): self.post_add(datasource) diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py index a7c77c2a512ee..2edf949397739 100644 --- a/superset/connectors/sqla/views.py +++ b/superset/connectors/sqla/views.py @@ -155,11 +155,11 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView): # noqa def post_add(self, metric): if metric.is_restricted: - security_manager.merge_perm('metric_access', metric.get_perm()) + security_manager.add_permission_view_menu('metric_access', metric.get_perm()) def post_update(self, metric): if metric.is_restricted: - security_manager.merge_perm('metric_access', metric.get_perm()) + security_manager.add_permission_view_menu('metric_access', metric.get_perm()) appbuilder.add_view_no_menu(SqlMetricInlineView) @@ -283,9 +283,9 @@ def pre_add(self, table): def post_add(self, table, flash_message=True): table.fetch_metadata() - security_manager.merge_perm('datasource_access', table.get_perm()) + security_manager.add_permission_view_menu('datasource_access', table.get_perm()) if table.schema: - security_manager.merge_perm('schema_access', table.schema_perm) + security_manager.add_permission_view_menu('schema_access', table.schema_perm) if flash_message: flash(_( diff --git a/superset/security.py b/superset/security.py index b30b2e516cbaa..f8ae0571560ec 100644 --- a/superset/security.py +++ b/superset/security.py @@ -263,26 +263,22 @@ def accessible_by_user(self, database, datasource_names, schema=None): return [d for d in datasource_names if d in full_names] def merge_perm(self, permission_name, view_menu_name): - # Implementation copied from sm.find_permission_view_menu. - # TODO: use sm.find_permission_view_menu once issue - # https://github.com/airbnb/superset/issues/1944 is resolved. - permission = self.find_permission(permission_name) - view_menu = self.find_view_menu(view_menu_name) - pv = None - if permission and view_menu: - pv = self.get_session.query(self.permissionview_model).filter_by( - permission=permission, view_menu=view_menu).first() - if not pv and permission_name and view_menu_name: - self.add_permission_view_menu(permission_name, view_menu_name) + logging.warning( + "This method 'merge_perm' is deprecated use add_permission_view_menu", + ) + self.add_permission_view_menu(permission_name, view_menu_name) def is_user_defined_permission(self, perm): return perm.permission.name in self.OBJECT_SPEC_PERMISSIONS def create_custom_permissions(self): # Global perms - self.merge_perm('all_datasource_access', 'all_datasource_access') - self.merge_perm('all_database_access', 'all_database_access') - self.merge_perm('can_only_access_owned_queries', 'can_only_access_owned_queries') + self.add_permission_view_menu('all_datasource_access', 'all_datasource_access') + self.add_permission_view_menu('all_database_access', 'all_database_access') + self.add_permission_view_menu( + 'can_only_access_owned_queries', + 'can_only_access_owned_queries', + ) def create_missing_perms(self): """Creates missing perms for datasources, schemas and metrics""" @@ -299,7 +295,7 @@ def create_missing_perms(self): def merge_pv(view_menu, perm): """Create permission view menu only if it doesn't exist""" if view_menu and perm and (view_menu, perm) not in all_pvs: - self.merge_perm(view_menu, perm) + self.add_permission_view_menu(view_menu, perm) logging.info('Creating missing datasource permissions.') datasources = ConnectorRegistry.get_all_datasources(db.session) diff --git a/superset/views/core.py b/superset/views/core.py index 1f69f4049f7ed..8f17ea4d82ae3 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -310,10 +310,10 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): # noqa def pre_add(self, db): self.check_extra(db) db.set_sqlalchemy_uri(db.sqlalchemy_uri) - security_manager.merge_perm('database_access', db.perm) + security_manager.add_permission_view_menu('database_access', db.perm) # adding a new database we always want to force refresh schema list for schema in db.all_schema_names(): - security_manager.merge_perm( + security_manager.add_permission_view_menu( 'schema_access', security_manager.get_schema_perm(db, schema)) def pre_update(self, db): diff --git a/tests/access_tests.py b/tests/access_tests.py index f608f00a3b25d..0bc1743cfe1ff 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -273,7 +273,7 @@ def test_clean_requests_after_db_grant(self): # gamma gets granted database access database = session.query(models.Database).first() - security_manager.merge_perm('database_access', database.perm) + security_manager.add_permission_view_menu('database_access', database.perm) ds_perm_view = security_manager.find_permission_view_menu( 'database_access', database.perm) security_manager.add_permission_role( @@ -309,7 +309,7 @@ def test_clean_requests_after_schema_grant(self): table_name='wb_health_population').first() ds.schema = 'temp_schema' - security_manager.merge_perm('schema_access', ds.schema_perm) + security_manager.add_permission_view_menu('schema_access', ds.schema_perm) schema_perm_view = security_manager.find_permission_view_menu( 'schema_access', ds.schema_perm) security_manager.add_permission_role( diff --git a/tests/druid_tests.py b/tests/druid_tests.py index 78d1bb8635d2b..164b16ad5f38c 100644 --- a/tests/druid_tests.py +++ b/tests/druid_tests.py @@ -289,8 +289,8 @@ def test_filter_druid_datasource(self): db.session.merge(no_gamma_ds) db.session.commit() - security_manager.merge_perm('datasource_access', gamma_ds.perm) - security_manager.merge_perm('datasource_access', no_gamma_ds.perm) + security_manager.add_permission_view_menu('datasource_access', gamma_ds.perm) + security_manager.add_permission_view_menu('datasource_access', no_gamma_ds.perm) perm = security_manager.find_permission_view_menu( 'datasource_access', gamma_ds.get_perm()) From 1ae000a26220c024f3f904d9049cfcd4b1e461e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20Math=C3=A9i?= Date: Tue, 21 May 2019 07:14:23 +0200 Subject: [PATCH 11/35] Boxplot should not require a datetime column (#5096) See https://github.com/apache/incubator-superset/issues/2222 --- superset/viz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/viz.py b/superset/viz.py index 9661e238f84c7..a9e3f93e4dc85 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -853,7 +853,7 @@ class BoxPlotViz(NVD3Viz): viz_type = 'box_plot' verbose_name = _('Box Plot') sort_series = False - is_timeseries = True + is_timeseries = False def to_series(self, df, classed='', title_suffix=''): label_sep = ' - ' From 551fe926cfe229853c10f6b620745574f75575e6 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 21 May 2019 13:21:05 -0700 Subject: [PATCH 12/35] Add dotenv to dependencies (#7330) --- requirements.txt | 1 + setup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/requirements.txt b/requirements.txt index 3076586a7947b..33c6441acc6d6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -63,6 +63,7 @@ pydruid==0.5.2 pyjwt==1.7.1 # via flask-appbuilder, flask-jwt-extended pyrsistent==0.14.11 # via jsonschema python-dateutil==2.6.1 +python-dotenv==0.10.1 python-editor==1.0.3 # via alembic python-geohash==0.8.5 python3-openid==3.1.0 # via flask-openid diff --git a/setup.py b/setup.py index ea58d48f19c31..6fc74880b4784 100644 --- a/setup.py +++ b/setup.py @@ -95,6 +95,7 @@ def get_git_sha(): 'polyline', 'pydruid>=0.5.2', 'python-dateutil', + 'python-dotenv', 'python-geohash', 'pyyaml>=3.13', 'requests>=2.20.0', From efb085ad038ee10f4c2f8e8d32c6dfca29b6e522 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Tue, 21 May 2019 16:02:46 -0700 Subject: [PATCH 13/35] [cypress] fix accessing a cross-origin frame error (#7552) --- superset/assets/cypress/integration/dashboard/save.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset/assets/cypress/integration/dashboard/save.js b/superset/assets/cypress/integration/dashboard/save.js index 772862d5b87e3..d0895777f48fa 100644 --- a/superset/assets/cypress/integration/dashboard/save.js +++ b/superset/assets/cypress/integration/dashboard/save.js @@ -20,6 +20,8 @@ import readResponseBlob from '../../utils/readResponseBlob'; import { WORLD_HEALTH_DASHBOARD } from './dashboard.helper'; export default () => describe('save', () => { + Cypress.config('chromeWebSecurity', false); + let dashboardId; let boxplotChartId; From 6b9790c8b198f4968cc6359f38a50e769f9ec09e Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Wed, 22 May 2019 09:41:25 -0700 Subject: [PATCH 14/35] [ad-hoc] Fixing type for count distinct (#7573) --- superset/connectors/druid/models.py | 2 +- tests/druid_func_tests.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 17ec4b82ebcf8..c71bc8061962c 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -932,7 +932,7 @@ def druid_type_from_adhoc_metric(adhoc_metric): if aggregate == 'count': return 'count' if aggregate == 'count_distinct': - return 'cardinality' + return 'hyperUnique' if column_type == 'hyperunique' else 'cardinality' else: return column_type + aggregate.capitalize() diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py index cf0c5e99fc925..2a7a5af8b7b5a 100644 --- a/tests/druid_func_tests.py +++ b/tests/druid_func_tests.py @@ -771,6 +771,13 @@ def test_druid_type_from_adhoc_metric(self): }) assert(druid_type == 'cardinality') + druid_type = DruidDatasource.druid_type_from_adhoc_metric({ + 'column': {'type': 'hyperUnique', 'column_name': 'value'}, + 'aggregate': 'COUNT_DISTINCT', + 'label': 'My Adhoc Metric', + }) + assert(druid_type == 'hyperUnique') + def test_run_query_order_by_metrics(self): client = Mock() client.query_builder.last_query.query_dict = {'mock': 0} From e5739fbbd2eb9150a5ec889cd38f588501945a43 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Wed, 22 May 2019 09:42:03 -0700 Subject: [PATCH 15/35] [testconn] Explicit closing engine connection (#7570) --- superset/views/core.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 8f17ea4d82ae3..0aedb6b8b4351 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. # pylint: disable=C,R,W +from contextlib import closing from datetime import datetime, timedelta import inspect import logging @@ -37,7 +38,7 @@ import pandas as pd import simplejson as json import sqlalchemy as sqla -from sqlalchemy import and_, create_engine, MetaData, or_, update +from sqlalchemy import and_, create_engine, MetaData, or_, select, update from sqlalchemy.engine.url import make_url from sqlalchemy.exc import IntegrityError from werkzeug.routing import BaseConverter @@ -1813,8 +1814,9 @@ def testconn(self): connect_args['configuration'] = configuration engine = create_engine(uri, **engine_params) - engine.connect() - return json_success(json.dumps(engine.table_names(), indent=4)) + + with closing(engine.connect()) as conn: + return json_success(json.dumps(conn.scalar(select([1])))) except Exception as e: logging.exception(e) return json_error_response(( From 421183d3f46c48215e88e9d7d285f2dc6c7ccfe6 Mon Sep 17 00:00:00 2001 From: michellethomas Date: Wed, 22 May 2019 16:39:01 -0700 Subject: [PATCH 16/35] Adding controls for verifying options (#7468) * Creating withVerification HOC * Updating to use componentDidMount and componentDidUpdate and adding propTypes * Adding tests to withVerification * Adding documentation to withVerification --- .../components/withVerification_spec.jsx | 106 ++++++++++++++++++ .../src/explore/components/controls/index.js | 4 + .../components/controls/withVerification.jsx | 88 +++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx create mode 100644 superset/assets/src/explore/components/controls/withVerification.jsx diff --git a/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx b/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx new file mode 100644 index 0000000000000..44377ea2669c3 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx @@ -0,0 +1,106 @@ +/** + * 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. + */ +import React from 'react'; +import sinon from 'sinon'; +import { shallow } from 'enzyme'; +import fetchMock from 'fetch-mock'; + +import MetricsControl from '../../../../src/explore/components/controls/MetricsControl'; +import withVerification from '../../../../src/explore/components/controls/withVerification'; + +const defaultProps = { + name: 'metrics', + label: 'Metrics', + value: undefined, + multi: true, + columns: [ + { type: 'VARCHAR(255)', column_name: 'source' }, + { type: 'VARCHAR(255)', column_name: 'target' }, + { type: 'DOUBLE', column_name: 'value' }, + ], + savedMetrics: [ + { metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' }, + { metric_name: 'avg__value', expression: 'AVG(energy_usage.value)' }, + ], + datasourceType: 'sqla', + getEndpoint: controlValues => `valid_metrics?data=${controlValues}`, +}; + +const VALID_METRIC = { metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' }; + +function setup(overrides) { + const onChange = sinon.spy(); + const props = { + onChange, + ...defaultProps, + ...overrides, + }; + const VerifiedControl = withVerification(MetricsControl, 'metric_name', 'savedMetrics'); + const wrapper = shallow(); + fetchMock.mock('glob:*/valid_metrics*', `["${VALID_METRIC.metric_name}"]`); + return { props, wrapper, onChange }; +} + +afterEach(fetchMock.restore); + +describe('VerifiedMetricsControl', () => { + + it('Gets valid options', () => { + const { wrapper } = setup(); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + expect(wrapper.state('validOptions')).toEqual([VALID_METRIC]); + fetchMock.reset(); + }, 0); + }); + + it('Returns verified options', () => { + const { wrapper } = setup(); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + const child = wrapper.find(MetricsControl); + expect(child.props().savedMetrics).toEqual([VALID_METRIC]); + fetchMock.reset(); + }, 0); + }); + + it('Makes no calls if endpoint is not set', () => { + const { wrapper } = setup({ + getEndpoint: () => null, + }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(0); + expect(wrapper.state('validOptions')).toEqual(new Set()); + fetchMock.reset(); + }, 0); + }); + + it('Calls endpoint if control values change', () => { + const { props, wrapper } = setup({ controlValues: { metrics: 'sum__value' } }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + fetchMock.reset(); + }, 0); + wrapper.setProps({ ...props, controlValues: { metrics: 'avg__value' } }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + fetchMock.reset(); + }, 0); + }); +}); diff --git a/superset/assets/src/explore/components/controls/index.js b/superset/assets/src/explore/components/controls/index.js index 32a8d449c36e9..a5800f2d11c69 100644 --- a/superset/assets/src/explore/components/controls/index.js +++ b/superset/assets/src/explore/components/controls/index.js @@ -40,6 +40,7 @@ import MetricsControl from './MetricsControl'; import AdhocFilterControl from './AdhocFilterControl'; import FilterPanel from './FilterPanel'; import FilterBoxItemControl from './FilterBoxItemControl'; +import withVerification from './withVerification'; const controlMap = { AnnotationLayerControl, @@ -66,5 +67,8 @@ const controlMap = { AdhocFilterControl, FilterPanel, FilterBoxItemControl, + MetricsControlVerifiedOptions: withVerification(MetricsControl, 'metric_name', 'savedMetrics'), + SelectControlVerifiedOptions: withVerification(SelectControl, 'column_name', 'options'), + AdhocFilterControlVerifiedOptions: withVerification(AdhocFilterControl, 'column_name', 'columns'), }; export default controlMap; diff --git a/superset/assets/src/explore/components/controls/withVerification.jsx b/superset/assets/src/explore/components/controls/withVerification.jsx new file mode 100644 index 0000000000000..8f1e549d0a00a --- /dev/null +++ b/superset/assets/src/explore/components/controls/withVerification.jsx @@ -0,0 +1,88 @@ +/** + * 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. + */ +import React from 'react'; +import { SupersetClient } from '@superset-ui/connection'; + +import { isEqual } from 'lodash'; + +export default function withVerification(WrappedComponent, optionLabel, optionsName) { + /* + * This function will verify control options before passing them to the control by calling an + * endpoint on mount and when the controlValues change. controlValues should be set in + * mapStateToProps that can be added as a control override along with getEndpoint. + */ + class withVerificationComponent extends React.Component { + constructor(props) { + super(props); + this.state = { + validOptions: new Set(), + hasRunVerification: false, + }; + + this.getValidOptions = this.getValidOptions.bind(this); + } + + componentDidMount() { + this.getValidOptions(); + } + + componentDidUpdate(prevProps) { + const { hasRunVerification } = this.state; + if (!isEqual(this.props.controlValues, prevProps.controlValues) || !hasRunVerification) { + this.getValidOptions(); + } + } + + getValidOptions() { + const endpoint = this.props.getEndpoint(this.props.controlValues); + if (endpoint) { + SupersetClient.get({ + endpoint, + }).then(({ json }) => { + if (Array.isArray(json)) { + this.setState({ validOptions: new Set(json) || new Set() }); + } + }).catch(error => console.log(error)); + + if (!this.state.hasRunVerification) { + this.setState({ hasRunVerification: true }); + } + } + } + + render() { + const { validOptions } = this.state; + const options = this.props[optionsName]; + const verifiedOptions = validOptions.size ? + options.filter(o => (validOptions.has(o[optionLabel]))) : + options; + + const newProps = { ...this.props, [optionsName]: verifiedOptions }; + + return ( + + ); + } + } + withVerificationComponent.propTypes = WrappedComponent.propTypes; + return withVerificationComponent; +} + From 9c8f494c9dfcc2bf32b5aca13d6fb943fb4e9857 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 23 May 2019 09:15:04 -0700 Subject: [PATCH 17/35] A few improvements to scheduling queries (#7585) * Better message for scheduling queries * Only allow scheduling after success * Ask for query name and description * Use CSS instead of
--- superset/assets/src/SqlLab/actions/sqlLab.js | 11 +++ .../SqlLab/components/ScheduleQueryButton.jsx | 68 +++++++++++++++++-- .../src/SqlLab/components/SqlEditor.jsx | 12 +++- .../SqlLab/components/TabbedSqlEditors.jsx | 4 ++ superset/config.py | 3 +- 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/superset/assets/src/SqlLab/actions/sqlLab.js b/superset/assets/src/SqlLab/actions/sqlLab.js index 81c8e8d5593ec..9c5a9d04826bb 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -107,6 +107,17 @@ export function saveQuery(query) { .catch(() => dispatch(addDangerToast(t('Your query could not be saved')))); } +export function scheduleQuery(query) { + return dispatch => + SupersetClient.post({ + endpoint: '/savedqueryviewapi/api/create', + postPayload: query, + stringify: false, + }) + .then(() => dispatch(addSuccessToast(t('Your query has been scheduled. To see details of your query, navigate to Saved Queries')))) + .catch(() => dispatch(addDangerToast(t('Your query could not be scheduled')))); +} + export function startQuery(query) { Object.assign(query, { id: query.id ? query.id : shortid.generate(), diff --git a/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx b/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx index 99ecaa5256052..d8f82fba62428 100644 --- a/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx +++ b/superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx @@ -20,6 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Form from 'react-jsonschema-form'; import chrono from 'chrono-node'; +import { Col, FormControl, FormGroup, Row } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; import Button from '../../components/Button'; @@ -76,11 +77,17 @@ const propTypes = { dbId: PropTypes.number.isRequired, animation: PropTypes.bool, onSchedule: PropTypes.func, + scheduleQueryWarning: PropTypes.string, + disabled: PropTypes.bool, + tooltip: PropTypes.string, }; const defaultProps = { defaultLabel: t('Undefined'), animation: true, onSchedule: () => {}, + scheduleQueryWarning: null, + disabled: false, + tooltip: null, }; class ScheduleQueryButton extends React.PureComponent { @@ -123,12 +130,53 @@ class ScheduleQueryButton extends React.PureComponent { } renderModalBody() { return ( -
+ + + + + + + + + + + + + + + + + + + {this.props.scheduleQueryWarning && ( + + + + {this.props.scheduleQueryWarning} + + + + )} + ); } render() { @@ -139,7 +187,13 @@ class ScheduleQueryButton extends React.PureComponent { modalTitle={t('Schedule Query')} modalBody={this.renderModalBody()} triggerNode={ - } diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx index 0b25dcf20ecad..a4aabb73fd49a 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -67,13 +67,14 @@ const propTypes = { defaultQueryLimit: PropTypes.number.isRequired, maxRow: PropTypes.number.isRequired, saveQueryWarning: PropTypes.string, + scheduleQueryWarning: PropTypes.string, }; const defaultProps = { database: null, latestQuery: null, hideLeftBar: false, - saveQueryWarning: null, + scheduleQueryWarning: null, }; class SqlEditor extends React.PureComponent { @@ -334,6 +335,10 @@ class SqlEditor extends React.PureComponent { ); } + const successful = this.props.latestQuery && this.props.latestQuery.state === 'success'; + const scheduleToolTip = successful + ? t('Schedule the query periodically') + : t('You must run the query successfully first'); return (
@@ -355,9 +360,12 @@ class SqlEditor extends React.PureComponent { defaultLabel={qe.title} sql={qe.sql} className="m-r-5" - onSchedule={this.props.actions.saveQuery} + onSchedule={this.props.actions.scheduleQuery} schema={qe.schema} dbId={qe.dbId} + scheduleQueryWarning={this.props.scheduleQueryWarning} + tooltip={scheduleToolTip} + disabled={!successful} /> } diff --git a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx index a8516f174387b..7c32021107e2d 100644 --- a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx +++ b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx @@ -41,11 +41,13 @@ const propTypes = { tables: PropTypes.array.isRequired, offline: PropTypes.bool, saveQueryWarning: PropTypes.string, + scheduleQueryWarning: PropTypes.string, }; const defaultProps = { queryEditors: [], offline: false, saveQueryWarning: null, + scheduleQueryWarning: null, }; let queryCount = 1; @@ -250,6 +252,7 @@ class TabbedSqlEditors extends React.PureComponent { defaultQueryLimit={this.props.defaultQueryLimit} maxRow={this.props.maxRow} saveQueryWarning={this.props.saveQueryWarning} + scheduleQueryWarning={this.props.scheduleQueryWarning} /> )} @@ -294,6 +297,7 @@ function mapStateToProps({ sqlLab, common }) { defaultQueryLimit: common.conf.DEFAULT_SQLLAB_LIMIT, maxRow: common.conf.SQL_MAX_ROW, saveQueryWarning: common.conf.SQLLAB_SAVE_WARNING_MESSAGE, + scheduleQueryWarning: common.conf.SQLLAB_SCHEDULE_WARNING_MESSAGE, }; } function mapDispatchToProps(dispatch) { diff --git a/superset/config.py b/superset/config.py index 5f9bd8a3bf2f9..fc287b7f2c13d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -357,8 +357,9 @@ # Maximum number of tables/views displayed in the dropdown window in SQL Lab. MAX_TABLE_NAMES = 3000 -# Adds a warning message on sqllab save query modal. +# Adds a warning message on sqllab save query and schedule query modals. SQLLAB_SAVE_WARNING_MESSAGE = None +SQLLAB_SCHEDULE_WARNING_MESSAGE = None # If defined, shows this text in an alert-warning box in the navbar # one example use case may be "STAGING" to make it clear that this is From f68f9790528107d389950bb3317365fd2847cf56 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 23 May 2019 09:17:00 -0700 Subject: [PATCH 18/35] Fix for polling queries (#7559) * Fix for polling queries * Revert changes due to https://github.com/apache/incubator-superset/pull/7575 --- superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx index 21d335b03edcc..6704d393fe754 100644 --- a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx +++ b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx @@ -40,13 +40,8 @@ class QueryAutoRefresh extends React.PureComponent { // if there are started or running queries, this method should return true const { queries } = this.props; const now = new Date().getTime(); - - // due to a race condition, queries can be marked as successful before the - // results key is set; this is a workaround until we fix the underlying - // problem const isQueryRunning = q => ( - ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0 || - (q.state === 'success' && q.resultsKey === null) + ['running', 'started', 'pending', 'fetching'].indexOf(q.state) >= 0 ); return ( From 265e117a4a95136f71a7806903de961fe507930e Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 23 May 2019 11:22:15 -0700 Subject: [PATCH 19/35] Add link to scheduled pipeline (#7584) * Add link to scheduled pipeline * Split utils into separate file * Fix unit test * Fix separator recursion --- docs/installation.rst | 6 ++ .../javascripts/showSavedQuery/utils_spec.jsx | 65 +++++++++++++++++++ superset/assets/src/showSavedQuery/index.css | 20 ++++++ superset/assets/src/showSavedQuery/index.jsx | 38 +++++++---- superset/assets/src/showSavedQuery/utils.js | 44 +++++++++++++ superset/views/sql_lab.py | 5 +- 6 files changed, 164 insertions(+), 14 deletions(-) create mode 100644 superset/assets/spec/javascripts/showSavedQuery/utils_spec.jsx create mode 100644 superset/assets/src/showSavedQuery/index.css create mode 100644 superset/assets/src/showSavedQuery/utils.js diff --git a/docs/installation.rst b/docs/installation.rst index 329debc8a0469..3e2934a37dff1 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -911,6 +911,12 @@ To allow scheduled queries, add the following to your `config.py`: 'container': 'end_date', }, ], + # link to the scheduler; this example links to an Airflow pipeline + # that uses the query id and the output table as its name + 'linkback': ( + 'https://airflow.example.com/admin/airflow/tree?' + 'dag_id=query_${id}_${extra_json.schedule_info.output_table}' + ), }, } diff --git a/superset/assets/spec/javascripts/showSavedQuery/utils_spec.jsx b/superset/assets/spec/javascripts/showSavedQuery/utils_spec.jsx new file mode 100644 index 0000000000000..d198a49180f6b --- /dev/null +++ b/superset/assets/spec/javascripts/showSavedQuery/utils_spec.jsx @@ -0,0 +1,65 @@ +/** + * 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. + */ +import { getNestedValue, interpolate } from '../../../src/showSavedQuery/utils'; + +describe('getNestedValue', () => { + it('is a function', () => { + expect(typeof getNestedValue).toBe('function'); + }); + + it('works with simple ids', () => { + const obj = { a: '1' }; + const id = 'a'; + expect(getNestedValue(obj, id)).toEqual('1'); + }); + + it('works with complex ids', () => { + const obj = { a: { b: '1' } }; + const id = 'a.b'; + expect(getNestedValue(obj, id)).toEqual('1'); + }); + + it('works with other separators', () => { + const obj = { a: { b: { c: '1' } } }; + const id = 'a__b__c'; + const separator = '__'; + expect(getNestedValue(obj, id, separator)).toEqual('1'); + }); +}); + + +describe('interpolate', () => { + it('is a function', () => { + expect(typeof interpolate).toBe('function'); + }); + + it('works with simple ids', () => { + const obj = { a: '1' }; + // eslint-disable-next-line no-template-curly-in-string + const str = 'value: ${a}'; + expect(interpolate(str, obj)).toEqual('value: 1'); + }); + + it('works with complex ids', () => { + const obj = { a: { b: '1' } }; + // eslint-disable-next-line no-template-curly-in-string + const str = 'value: ${a.b}'; + expect(interpolate(str, obj)).toEqual('value: 1'); + }); +}); diff --git a/superset/assets/src/showSavedQuery/index.css b/superset/assets/src/showSavedQuery/index.css new file mode 100644 index 0000000000000..026dd784f34ca --- /dev/null +++ b/superset/assets/src/showSavedQuery/index.css @@ -0,0 +1,20 @@ +/** + * 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. + */ +.btn-add { display: none; } +.linkback { padding: 0 10px 20px 2px; } diff --git a/superset/assets/src/showSavedQuery/index.jsx b/superset/assets/src/showSavedQuery/index.jsx index cd384fe5b461e..c64626cb6a4f9 100644 --- a/superset/assets/src/showSavedQuery/index.jsx +++ b/superset/assets/src/showSavedQuery/index.jsx @@ -19,25 +19,39 @@ import React from 'react'; import ReactDom from 'react-dom'; import Form from 'react-jsonschema-form'; +import { interpolate } from 'src/showSavedQuery/utils'; +import './index.css'; const scheduleInfoContainer = document.getElementById('schedule-info'); const bootstrapData = JSON.parse(scheduleInfoContainer.getAttribute('data-bootstrap')); -const schemas = bootstrapData.common.feature_flags.SCHEDULED_QUERIES; -const scheduleInfo = bootstrapData.common.extra_json.schedule_info; +const config = bootstrapData.common.feature_flags.SCHEDULED_QUERIES; +const query = bootstrapData.common.query; +const scheduleInfo = query.extra_json.schedule_info; +const linkback = config.linkback + ? interpolate(config.linkback, query) + : null; -if (scheduleInfo && schemas) { +if (scheduleInfo && config) { // hide instructions when showing schedule info - schemas.JSONSCHEMA.description = ''; + config.JSONSCHEMA.description = ''; ReactDom.render( - -
- , +
+
+
+
+ {linkback && } +
, scheduleInfoContainer, ); } diff --git a/superset/assets/src/showSavedQuery/utils.js b/superset/assets/src/showSavedQuery/utils.js new file mode 100644 index 0000000000000..9cd712bc893f3 --- /dev/null +++ b/superset/assets/src/showSavedQuery/utils.js @@ -0,0 +1,44 @@ +/** + * 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. + */ + +export function getNestedValue(obj, id, separator = '.') { + /* + * Given a nested object and an id, return the nested value. + * + * > getNestedValue({a:{b:1}}, 'a.b') + * < 1 + */ + const index = id.indexOf(separator); + if (index === -1) { + return obj[id]; + } + const name = id.slice(0, index); + const rest = id.slice(index + separator.length); + return getNestedValue(obj[name], rest, separator); +} + +export function interpolate(str, obj) { + /* + * Programmatic template string for interpolation. + * + * > interpolate('foo ${a.b}', {a:{b:1}}) + * < "foo 1" + */ + return str.replace(/\$\{(.+?)\}/g, (match, id) => getNestedValue(obj, id)); +} diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py index 37c84c2705253..98046f891d449 100644 --- a/superset/views/sql_lab.py +++ b/superset/views/sql_lab.py @@ -122,11 +122,12 @@ def pre_update(self, obj): def show(self, pk): pk = self._deserialize_pk_if_composite(pk) widgets = self._show(pk) - extra_json = self.datamodel.get(pk).extra_json + query = self.datamodel.get(pk).to_json() + query['extra_json'] = json.loads(query['extra_json']) payload = { 'common': { 'feature_flags': get_feature_flags(), - 'extra_json': json.loads(extra_json), + 'query': query, }, } From e4f8444f8200a9aad2750b8f048e0c6f92aeddd3 Mon Sep 17 00:00:00 2001 From: TheLastSultan <41172775+TheLastSultan@users.noreply.github.com> Date: Thu, 23 May 2019 17:32:16 -0700 Subject: [PATCH 20/35] add American Express to list of users on readme (#7576) --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 6b13d4e1fb5e4..962c8c9bfb26b 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,7 @@ the world know they are using Superset. Join our growing community! 1. [Airbnb](https://github.com/airbnb) 1. [Airboxlab](https://foobot.io) 1. [Aktia Bank plc](https://www.aktia.com) + 1. [American Express](https://www.americanexpress.com) 1. [Amino](https://amino.com) 1. [Apollo GraphQL](https://www.apollographql.com/) 1. [Ascendica Development](http://ascendicadevelopment.com) From 20143293eb9cac93a1692dcd552be5cb48b5d3e9 Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Thu, 23 May 2019 17:39:40 -0700 Subject: [PATCH 21/35] [sql lab]revert #4833 (#7498) --- superset/views/core.py | 47 +++++++----------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 0aedb6b8b4351..883a2d95a92ca 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -21,7 +21,6 @@ import logging import os import re -import time import traceback from typing import List # noqa: F401 from urllib import parse @@ -37,8 +36,7 @@ from flask_babel import lazy_gettext as _ import pandas as pd import simplejson as json -import sqlalchemy as sqla -from sqlalchemy import and_, create_engine, MetaData, or_, select, update +from sqlalchemy import and_, create_engine, MetaData, or_, select from sqlalchemy.engine.url import make_url from sqlalchemy.exc import IntegrityError from werkzeug.routing import BaseConverter @@ -1846,7 +1844,7 @@ def recent_activity(self, user_id): M.Slice.id == M.Log.slice_id, ) .filter( - sqla.and_( + and_( ~M.Log.action.in_(('queries', 'shortner', 'sql_json')), M.Log.user_id == user_id, ), @@ -1916,7 +1914,7 @@ def fave_dashboards(self, user_id): ) .join( models.FavStar, - sqla.and_( + and_( models.FavStar.user_id == int(user_id), models.FavStar.class_name == 'Dashboard', models.Dashboard.id == models.FavStar.obj_id, @@ -1954,7 +1952,7 @@ def created_dashboards(self, user_id): Dash, ) .filter( - sqla.or_( + or_( Dash.created_by_fk == user_id, Dash.changed_by_fk == user_id, ), @@ -1987,13 +1985,13 @@ def user_slices(self, user_id=None): db.session.query(Slice, FavStar.dttm).join( models.FavStar, - sqla.and_( + and_( models.FavStar.user_id == int(user_id), models.FavStar.class_name == 'slice', models.Slice.id == models.FavStar.obj_id, ), isouter=True).filter( - sqla.or_( + or_( Slice.created_by_fk == user_id, Slice.changed_by_fk == user_id, FavStar.user_id == user_id, @@ -2024,7 +2022,7 @@ def created_slices(self, user_id=None): qry = ( db.session.query(Slice) .filter( - sqla.or_( + or_( Slice.created_by_fk == user_id, Slice.changed_by_fk == user_id, ), @@ -2056,7 +2054,7 @@ def fave_slices(self, user_id=None): ) .join( models.FavStar, - sqla.and_( + and_( models.FavStar.user_id == int(user_id), models.FavStar.class_name == 'slice', models.Slice.id == models.FavStar.obj_id, @@ -2803,35 +2801,6 @@ def queries(self, last_updated_ms): .all() ) dict_queries = {q.client_id: q.to_dict() for q in sql_queries} - - now = int(round(time.time() * 1000)) - - unfinished_states = [ - QueryStatus.PENDING, - QueryStatus.RUNNING, - ] - - queries_to_timeout = [ - client_id for client_id, query_dict in dict_queries.items() - if ( - query_dict['state'] in unfinished_states and ( - now - query_dict['startDttm'] > - config.get('SQLLAB_ASYNC_TIME_LIMIT_SEC') * 1000 - ) - ) - ] - - if queries_to_timeout: - update(Query).where( - and_( - Query.user_id == g.user.get_id(), - Query.client_id in queries_to_timeout, - ), - ).values(state=QueryStatus.TIMED_OUT) - - for client_id in queries_to_timeout: - dict_queries[client_id]['status'] = QueryStatus.TIMED_OUT - return json_success( json.dumps(dict_queries, default=utils.json_int_dttm_ser)) From 47ba2ad3947a83bb5299c9f80a9aedd4a756a004 Mon Sep 17 00:00:00 2001 From: michellethomas Date: Fri, 24 May 2019 11:10:35 -0700 Subject: [PATCH 22/35] Remove aggregates from metric options if datasource has no columns (#7586) --- .../explore/components/MetricsControl_spec.jsx | 8 ++++++++ .../src/explore/components/controls/MetricsControl.jsx | 10 +++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx index 6f2c657a485ff..31bddf87dd686 100644 --- a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx +++ b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx @@ -85,6 +85,14 @@ describe('MetricsControl', () => { ]); }); + it('does not show aggregates in options if no columns', () => { + const { wrapper } = setup({ columns: [] }); + expect(wrapper.state('options')).toEqual([ + { optionName: 'sum__value', metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' }, + { optionName: 'avg__value', metric_name: 'avg__value', expression: 'AVG(energy_usage.value)' }, + ]); + }); + it('coerces Adhoc Metrics from form data into instances of the AdhocMetric class and leaves saved metrics', () => { const { wrapper } = setup({ value: [ diff --git a/superset/assets/src/explore/components/controls/MetricsControl.jsx b/superset/assets/src/explore/components/controls/MetricsControl.jsx index b42cc814d67b6..1e49355e3d841 100644 --- a/superset/assets/src/explore/components/controls/MetricsControl.jsx +++ b/superset/assets/src/explore/components/controls/MetricsControl.jsx @@ -238,10 +238,14 @@ export default class MetricsControl extends React.PureComponent { } optionsForSelect(props) { + const { columns, savedMetrics } = props; + const aggregates = columns && columns.length ? + Object.keys(AGGREGATES).map(aggregate => ({ aggregate_name: aggregate })) : + []; const options = [ - ...props.columns, - ...Object.keys(AGGREGATES).map(aggregate => ({ aggregate_name: aggregate })), - ...props.savedMetrics, + ...columns, + ...aggregates, + ...savedMetrics, ]; return options.reduce((results, option) => { From f7d3413a501d8b643318fe7c0641eba608a079f5 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Sun, 26 May 2019 06:13:16 +0300 Subject: [PATCH 23/35] Add support for period character in table names (#7453) * Move schema name handling in table names from frontend to backend * Rename all_schema_names to get_all_schema_names * Fix js errors * Fix additional js linting errors * Refactor datasource getters and fix linting errors * Update js unit tests * Add python unit test for get_table_names method * Add python unit test for get_table_names method * Fix js linting error --- .../components/TableSelector_spec.jsx | 11 +-- .../spec/javascripts/sqllab/fixtures.js | 6 +- .../SqlLab/components/SqlEditorLeftBar.jsx | 15 +--- .../assets/src/components/TableSelector.jsx | 9 +- superset/cli.py | 4 +- superset/db_engine_specs.py | 84 +++++++++---------- superset/models/core.py | 57 +++++-------- superset/security.py | 6 +- superset/utils/core.py | 7 +- superset/views/core.py | 67 ++++++++------- tests/db_engine_specs_test.py | 19 +++++ 11 files changed, 148 insertions(+), 137 deletions(-) diff --git a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx index 70e2cca1f1925..13665927556d3 100644 --- a/superset/assets/spec/javascripts/components/TableSelector_spec.jsx +++ b/superset/assets/spec/javascripts/components/TableSelector_spec.jsx @@ -208,19 +208,20 @@ describe('TableSelector', () => { it('test 1', () => { wrapper.instance().changeTable({ - value: 'birth_names', + value: { schema: 'main', table: 'birth_names' }, label: 'birth_names', }); expect(wrapper.state().tableName).toBe('birth_names'); }); - it('test 2', () => { + it('should call onTableChange with schema from table object', () => { + wrapper.setProps({ schema: null }); wrapper.instance().changeTable({ - value: 'main.my_table', - label: 'my_table', + value: { schema: 'other_schema', table: 'my_table' }, + label: 'other_schema.my_table', }); expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table'); - expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main'); + expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema'); }); }); diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index 6471be1286556..f43f43f550a68 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -329,15 +329,15 @@ export const databases = { export const tables = { options: [ { - value: 'birth_names', + value: { schema: 'main', table: 'birth_names' }, label: 'birth_names', }, { - value: 'energy_usage', + value: { schema: 'main', table: 'energy_usage' }, label: 'energy_usage', }, { - value: 'wb_health_population', + value: { schema: 'main', table: 'wb_health_population' }, label: 'wb_health_population', }, ], diff --git a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx index 9d0796cd8ce74..43ea4873a0f73 100644 --- a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx @@ -83,17 +83,10 @@ export default class SqlEditorLeftBar extends React.PureComponent { this.setState({ tableName: '' }); return; } - const namePieces = tableOpt.value.split('.'); - let tableName = namePieces[0]; - let schemaName = this.props.queryEditor.schema; - if (namePieces.length === 1) { - this.setState({ tableName }); - } else { - schemaName = namePieces[0]; - tableName = namePieces[1]; - this.setState({ tableName }); - this.props.actions.queryEditorSetSchema(this.props.queryEditor, schemaName); - } + const schemaName = tableOpt.value.schema; + const tableName = tableOpt.value.table; + this.setState({ tableName }); + this.props.actions.queryEditorSetSchema(this.props.queryEditor, schemaName); this.props.actions.addTable(this.props.queryEditor, tableName, schemaName); } diff --git a/superset/assets/src/components/TableSelector.jsx b/superset/assets/src/components/TableSelector.jsx index ba2cebb2799d8..940e1c274b93a 100644 --- a/superset/assets/src/components/TableSelector.jsx +++ b/superset/assets/src/components/TableSelector.jsx @@ -170,13 +170,8 @@ export default class TableSelector extends React.PureComponent { this.setState({ tableName: '' }); return; } - const namePieces = tableOpt.value.split('.'); - let tableName = namePieces[0]; - let schemaName = this.props.schema; - if (namePieces.length > 1) { - schemaName = namePieces[0]; - tableName = namePieces[1]; - } + const schemaName = tableOpt.value.schema; + const tableName = tableOpt.value.table; if (this.props.tableNameSticky) { this.setState({ tableName }, this.onChange); } diff --git a/superset/cli.py b/superset/cli.py index 7b441b47bdf9e..edb0102400571 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -288,9 +288,9 @@ def update_datasources_cache(): if database.allow_multi_schema_metadata_fetch: print('Fetching {} datasources ...'.format(database.name)) try: - database.all_table_names_in_database( + database.get_all_table_names_in_database( force=True, cache=True, cache_timeout=24 * 60 * 60) - database.all_view_names_in_database( + database.get_all_view_names_in_database( force=True, cache=True, cache_timeout=24 * 60 * 60) except Exception as e: print('{}'.format(str(e))) diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 35a591fa10202..67aba1264b9e5 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -122,6 +122,7 @@ class BaseEngineSpec(object): force_column_alias_quotes = False arraysize = 0 max_column_name_length = 0 + try_remove_schema_from_table_name = True @classmethod def get_time_expr(cls, expr, pdf, time_grain, grain): @@ -279,33 +280,32 @@ def convert_dttm(cls, target_type, dttm): return "'{}'".format(dttm.strftime('%Y-%m-%d %H:%M:%S')) @classmethod - def fetch_result_sets(cls, db, datasource_type): - """Returns a list of tables [schema1.table1, schema2.table2, ...] + def get_all_datasource_names(cls, db, datasource_type: str) \ + -> List[utils.DatasourceName]: + """Returns a list of all tables or views in database. - Datasource_type can be 'table' or 'view'. - Empty schema corresponds to the list of full names of the all - tables or views: .. + :param db: Database instance + :param datasource_type: Datasource_type can be 'table' or 'view' + :return: List of all datasources in database or schema """ - schemas = db.all_schema_names(cache=db.schema_cache_enabled, - cache_timeout=db.schema_cache_timeout, - force=True) - all_result_sets = [] + schemas = db.get_all_schema_names(cache=db.schema_cache_enabled, + cache_timeout=db.schema_cache_timeout, + force=True) + all_datasources: List[utils.DatasourceName] = [] for schema in schemas: if datasource_type == 'table': - all_datasource_names = db.all_table_names_in_schema( + all_datasources += db.get_all_table_names_in_schema( schema=schema, force=True, cache=db.table_cache_enabled, cache_timeout=db.table_cache_timeout) elif datasource_type == 'view': - all_datasource_names = db.all_view_names_in_schema( + all_datasources += db.get_all_view_names_in_schema( schema=schema, force=True, cache=db.table_cache_enabled, cache_timeout=db.table_cache_timeout) else: raise Exception(f'Unsupported datasource_type: {datasource_type}') - all_result_sets += [ - '{}.{}'.format(schema, t) for t in all_datasource_names] - return all_result_sets + return all_datasources @classmethod def handle_cursor(cls, cursor, query, session): @@ -352,11 +352,17 @@ def get_schema_names(cls, inspector): @classmethod def get_table_names(cls, inspector, schema): - return sorted(inspector.get_table_names(schema)) + tables = inspector.get_table_names(schema) + if schema and cls.try_remove_schema_from_table_name: + tables = [re.sub(f'^{schema}\\.', '', table) for table in tables] + return sorted(tables) @classmethod def get_view_names(cls, inspector, schema): - return sorted(inspector.get_view_names(schema)) + views = inspector.get_view_names(schema) + if schema and cls.try_remove_schema_from_table_name: + views = [re.sub(f'^{schema}\\.', '', view) for view in views] + return sorted(views) @classmethod def get_columns(cls, inspector: Inspector, table_name: str, schema: str) -> list: @@ -528,6 +534,7 @@ def convert_dttm(cls, target_type, dttm): class PostgresEngineSpec(PostgresBaseEngineSpec): engine = 'postgresql' max_column_name_length = 63 + try_remove_schema_from_table_name = False @classmethod def get_table_names(cls, inspector, schema): @@ -685,29 +692,25 @@ def epoch_to_dttm(cls): return "datetime({col}, 'unixepoch')" @classmethod - def fetch_result_sets(cls, db, datasource_type): - schemas = db.all_schema_names(cache=db.schema_cache_enabled, - cache_timeout=db.schema_cache_timeout, - force=True) - all_result_sets = [] + def get_all_datasource_names(cls, db, datasource_type: str) \ + -> List[utils.DatasourceName]: + schemas = db.get_all_schema_names(cache=db.schema_cache_enabled, + cache_timeout=db.schema_cache_timeout, + force=True) schema = schemas[0] if datasource_type == 'table': - all_datasource_names = db.all_table_names_in_schema( + return db.get_all_table_names_in_schema( schema=schema, force=True, cache=db.table_cache_enabled, cache_timeout=db.table_cache_timeout) elif datasource_type == 'view': - all_datasource_names = db.all_view_names_in_schema( + return db.get_all_view_names_in_schema( schema=schema, force=True, cache=db.table_cache_enabled, cache_timeout=db.table_cache_timeout) else: raise Exception(f'Unsupported datasource_type: {datasource_type}') - all_result_sets += [ - '{}.{}'.format(schema, t) for t in all_datasource_names] - return all_result_sets - @classmethod def convert_dttm(cls, target_type, dttm): iso = dttm.isoformat().replace('T', ' ') @@ -1107,24 +1110,19 @@ def epoch_to_dttm(cls): return 'from_unixtime({col})' @classmethod - def fetch_result_sets(cls, db, datasource_type): - """Returns a list of tables [schema1.table1, schema2.table2, ...] - - Datasource_type can be 'table' or 'view'. - Empty schema corresponds to the list of full names of the all - tables or views: .. - """ - result_set_df = db.get_df( + def get_all_datasource_names(cls, db, datasource_type: str) \ + -> List[utils.DatasourceName]: + datasource_df = db.get_df( """SELECT table_schema, table_name FROM INFORMATION_SCHEMA.{}S ORDER BY concat(table_schema, '.', table_name)""".format( datasource_type.upper(), ), None) - result_sets = [] - for unused, row in result_set_df.iterrows(): - result_sets.append('{}.{}'.format( - row['table_schema'], row['table_name'])) - return result_sets + datasource_names: List[utils.DatasourceName] = [] + for unused, row in datasource_df.iterrows(): + datasource_names.append(utils.DatasourceName( + schema=row['table_schema'], table=row['table_name'])) + return datasource_names @classmethod def extra_table_metadata(cls, database, table_name, schema_name): @@ -1385,9 +1383,9 @@ def patch(cls): hive.Cursor.fetch_logs = patched_hive.fetch_logs @classmethod - def fetch_result_sets(cls, db, datasource_type): - return BaseEngineSpec.fetch_result_sets( - db, datasource_type) + def get_all_datasource_names(cls, db, datasource_type: str) \ + -> List[utils.DatasourceName]: + return BaseEngineSpec.get_all_datasource_names(db, datasource_type) @classmethod def fetch_data(cls, cursor, limit): diff --git a/superset/models/core.py b/superset/models/core.py index e16a234bfd723..047a3ddb11b11 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -23,6 +23,7 @@ import json import logging import textwrap +from typing import List from flask import escape, g, Markup, request from flask_appbuilder import Model @@ -65,6 +66,7 @@ PASSWORD_MASK = 'X' * 10 + def set_related_perm(mapper, connection, target): # noqa src_class = target.cls_model id_ = target.datasource_id @@ -184,7 +186,7 @@ def clone(self): description=self.description, cache_timeout=self.cache_timeout) - @datasource.getter + @datasource.getter # type: ignore @utils.memoized def get_datasource(self): return ( @@ -210,7 +212,7 @@ def datasource_edit_url(self): datasource = self.datasource return datasource.url if datasource else None - @property + @property # type: ignore @utils.memoized def viz(self): d = json.loads(self.params) @@ -930,100 +932,87 @@ def inspector(self): @cache_util.memoized_func( key=lambda *args, **kwargs: 'db:{}:schema:None:table_list', attribute_in_key='id') - def all_table_names_in_database(self, cache=False, - cache_timeout=None, force=False): + def get_all_table_names_in_database(self, cache: bool = False, + cache_timeout: bool = None, + force=False) -> List[utils.DatasourceName]: """Parameters need to be passed as keyword arguments.""" if not self.allow_multi_schema_metadata_fetch: return [] - return self.db_engine_spec.fetch_result_sets(self, 'table') + return self.db_engine_spec.get_all_datasource_names(self, 'table') @cache_util.memoized_func( key=lambda *args, **kwargs: 'db:{}:schema:None:view_list', attribute_in_key='id') - def all_view_names_in_database(self, cache=False, - cache_timeout=None, force=False): + def get_all_view_names_in_database(self, cache: bool = False, + cache_timeout: bool = None, + force: bool = False) -> List[utils.DatasourceName]: """Parameters need to be passed as keyword arguments.""" if not self.allow_multi_schema_metadata_fetch: return [] - return self.db_engine_spec.fetch_result_sets(self, 'view') + return self.db_engine_spec.get_all_datasource_names(self, 'view') @cache_util.memoized_func( key=lambda *args, **kwargs: 'db:{{}}:schema:{}:table_list'.format( kwargs.get('schema')), attribute_in_key='id') - def all_table_names_in_schema(self, schema, cache=False, - cache_timeout=None, force=False): + def get_all_table_names_in_schema(self, schema: str, cache: bool = False, + cache_timeout: int = None, force: bool = False): """Parameters need to be passed as keyword arguments. For unused parameters, they are referenced in cache_util.memoized_func decorator. :param schema: schema name - :type schema: str :param cache: whether cache is enabled for the function - :type cache: bool :param cache_timeout: timeout in seconds for the cache - :type cache_timeout: int :param force: whether to force refresh the cache - :type force: bool - :return: table list - :rtype: list + :return: list of tables """ - tables = [] try: tables = self.db_engine_spec.get_table_names( inspector=self.inspector, schema=schema) + return [utils.DatasourceName(table=table, schema=schema) for table in tables] except Exception as e: logging.exception(e) - return tables @cache_util.memoized_func( key=lambda *args, **kwargs: 'db:{{}}:schema:{}:view_list'.format( kwargs.get('schema')), attribute_in_key='id') - def all_view_names_in_schema(self, schema, cache=False, - cache_timeout=None, force=False): + def get_all_view_names_in_schema(self, schema: str, cache: bool = False, + cache_timeout: int = None, force: bool = False): """Parameters need to be passed as keyword arguments. For unused parameters, they are referenced in cache_util.memoized_func decorator. :param schema: schema name - :type schema: str :param cache: whether cache is enabled for the function - :type cache: bool :param cache_timeout: timeout in seconds for the cache - :type cache_timeout: int :param force: whether to force refresh the cache - :type force: bool - :return: view list - :rtype: list + :return: list of views """ - views = [] try: views = self.db_engine_spec.get_view_names( inspector=self.inspector, schema=schema) + return [utils.DatasourceName(table=view, schema=schema) for view in views] except Exception as e: logging.exception(e) - return views @cache_util.memoized_func( key=lambda *args, **kwargs: 'db:{}:schema_list', attribute_in_key='id') - def all_schema_names(self, cache=False, cache_timeout=None, force=False): + def get_all_schema_names(self, cache: bool = False, cache_timeout: int = None, + force: bool = False) -> List[str]: """Parameters need to be passed as keyword arguments. For unused parameters, they are referenced in cache_util.memoized_func decorator. :param cache: whether cache is enabled for the function - :type cache: bool :param cache_timeout: timeout in seconds for the cache - :type cache_timeout: int :param force: whether to force refresh the cache - :type force: bool :return: schema list - :rtype: list """ return self.db_engine_spec.get_schema_names(self.inspector) @@ -1232,7 +1221,7 @@ def username(self): def datasource(self): return self.get_datasource - @datasource.getter + @datasource.getter # type: ignore @utils.memoized def get_datasource(self): # pylint: disable=no-member diff --git a/superset/security.py b/superset/security.py index f8ae0571560ec..89eab5d53bf16 100644 --- a/superset/security.py +++ b/superset/security.py @@ -17,6 +17,7 @@ # pylint: disable=C,R,W """A set of constants and methods to manage permissions and security""" import logging +from typing import List from flask import g from flask_appbuilder.security.sqla import models as ab_models @@ -26,6 +27,7 @@ from superset import sql_parse from superset.connectors.connector_registry import ConnectorRegistry from superset.exceptions import SupersetSecurityException +from superset.utils.core import DatasourceName class SupersetSecurityManager(SecurityManager): @@ -240,7 +242,9 @@ def schemas_accessible_by_user(self, database, schemas, hierarchical=True): subset.add(t.schema) return sorted(list(subset)) - def accessible_by_user(self, database, datasource_names, schema=None): + def get_datasources_accessible_by_user( + self, database, datasource_names: List[DatasourceName], + schema: str = None) -> List[DatasourceName]: from superset import db if self.database_access(database) or self.all_datasource_access(): return datasource_names diff --git a/superset/utils/core.py b/superset/utils/core.py index 3b4145793939a..2defa70dd179e 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -32,7 +32,7 @@ import smtplib import sys from time import struct_time -from typing import List, Optional, Tuple +from typing import List, NamedTuple, Optional, Tuple from urllib.parse import unquote_plus import uuid import zlib @@ -1100,3 +1100,8 @@ def MediumText() -> Variant: def shortid() -> str: return '{}'.format(uuid.uuid4())[-12:] + + +class DatasourceName(NamedTuple): + table: str + schema: str diff --git a/superset/views/core.py b/superset/views/core.py index 883a2d95a92ca..0a6ddefa18e04 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -22,7 +22,7 @@ import os import re import traceback -from typing import List # noqa: F401 +from typing import Dict, List # noqa: F401 from urllib import parse from flask import ( @@ -311,7 +311,7 @@ def pre_add(self, db): db.set_sqlalchemy_uri(db.sqlalchemy_uri) security_manager.add_permission_view_menu('database_access', db.perm) # adding a new database we always want to force refresh schema list - for schema in db.all_schema_names(): + for schema in db.get_all_schema_names(): security_manager.add_permission_view_menu( 'schema_access', security_manager.get_schema_perm(db, schema)) @@ -1545,7 +1545,7 @@ def schemas(self, db_id, force_refresh='false'): .first() ) if database: - schemas = database.all_schema_names( + schemas = database.get_all_schema_names( cache=database.schema_cache_enabled, cache_timeout=database.schema_cache_timeout, force=force_refresh) @@ -1570,50 +1570,57 @@ def tables(self, db_id, schema, substr, force_refresh='false'): database = db.session.query(models.Database).filter_by(id=db_id).one() if schema: - table_names = database.all_table_names_in_schema( + tables = database.get_all_table_names_in_schema( schema=schema, force=force_refresh, cache=database.table_cache_enabled, - cache_timeout=database.table_cache_timeout) - view_names = database.all_view_names_in_schema( + cache_timeout=database.table_cache_timeout) or [] + views = database.get_all_view_names_in_schema( schema=schema, force=force_refresh, cache=database.table_cache_enabled, - cache_timeout=database.table_cache_timeout) + cache_timeout=database.table_cache_timeout) or [] else: - table_names = database.all_table_names_in_database( + tables = database.get_all_table_names_in_database( cache=True, force=False, cache_timeout=24 * 60 * 60) - view_names = database.all_view_names_in_database( + views = database.get_all_view_names_in_database( cache=True, force=False, cache_timeout=24 * 60 * 60) - table_names = security_manager.accessible_by_user(database, table_names, schema) - view_names = security_manager.accessible_by_user(database, view_names, schema) + tables = security_manager.get_datasources_accessible_by_user( + database, tables, schema) + views = security_manager.get_datasources_accessible_by_user( + database, views, schema) + + def get_datasource_label(ds_name: utils.DatasourceName) -> str: + return ds_name.table if schema else f'{ds_name.schema}.{ds_name.table}' if substr: - table_names = [tn for tn in table_names if substr in tn] - view_names = [vn for vn in view_names if substr in vn] + tables = [tn for tn in tables if substr in get_datasource_label(tn)] + views = [vn for vn in views if substr in get_datasource_label(vn)] if not schema and database.default_schemas: - def get_schema(tbl_or_view_name): - return tbl_or_view_name.split('.')[0] if '.' in tbl_or_view_name else None - user_schema = g.user.email.split('@')[0] valid_schemas = set(database.default_schemas + [user_schema]) - table_names = [tn for tn in table_names if get_schema(tn) in valid_schemas] - view_names = [vn for vn in view_names if get_schema(vn) in valid_schemas] + tables = [tn for tn in tables if tn.schema in valid_schemas] + views = [vn for vn in views if vn.schema in valid_schemas] - max_items = config.get('MAX_TABLE_NAMES') or len(table_names) - total_items = len(table_names) + len(view_names) - max_tables = len(table_names) - max_views = len(view_names) + max_items = config.get('MAX_TABLE_NAMES') or len(tables) + total_items = len(tables) + len(views) + max_tables = len(tables) + max_views = len(views) if total_items and substr: - max_tables = max_items * len(table_names) // total_items - max_views = max_items * len(view_names) // total_items - - table_options = [{'value': tn, 'label': tn} - for tn in table_names[:max_tables]] - table_options.extend([{'value': vn, 'label': '[view] {}'.format(vn)} - for vn in view_names[:max_views]]) + max_tables = max_items * len(tables) // total_items + max_views = max_items * len(views) // total_items + + def get_datasource_value(ds_name: utils.DatasourceName) -> Dict[str, str]: + return {'schema': ds_name.schema, 'table': ds_name.table} + + table_options = [{'value': get_datasource_value(tn), + 'label': get_datasource_label(tn)} + for tn in tables[:max_tables]] + table_options.extend([{'value': get_datasource_value(vn), + 'label': f'[view] {get_datasource_label(vn)}'} + for vn in views[:max_views]]) payload = { - 'tableLength': len(table_names) + len(view_names), + 'tableLength': len(tables) + len(views), 'options': table_options, } return json_success(json.dumps(payload)) diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index e0d914f0b51e4..e190014e1e909 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -464,3 +464,22 @@ def test_mssql_where_clause_n_prefix(self): query = str(sel.compile(dialect=dialect, compile_kwargs={'literal_binds': True})) query_expected = "SELECT col, unicode_col \nFROM tbl \nWHERE col = 'abc' AND unicode_col = N'abc'" # noqa self.assertEqual(query, query_expected) + + def test_get_table_names(self): + inspector = mock.Mock() + inspector.get_table_names = mock.Mock(return_value=['schema.table', 'table_2']) + inspector.get_foreign_table_names = mock.Mock(return_value=['table_3']) + + """ Make sure base engine spec removes schema name from table name + ie. when try_remove_schema_from_table_name == True. """ + base_result_expected = ['table', 'table_2'] + base_result = db_engine_specs.BaseEngineSpec.get_table_names( + schema='schema', inspector=inspector) + self.assertListEqual(base_result_expected, base_result) + + """ Make sure postgres doesn't try to remove schema name from table name + ie. when try_remove_schema_from_table_name == False. """ + pg_result_expected = ['schema.table', 'table_2', 'table_3'] + pg_result = db_engine_specs.PostgresEngineSpec.get_table_names( + schema='schema', inspector=inspector) + self.assertListEqual(pg_result_expected, pg_result) From b21f8ec80436487e5778bcacd12205f6a2cfe46c Mon Sep 17 00:00:00 2001 From: Luca Toscano Date: Tue, 28 May 2019 23:19:04 +0200 Subject: [PATCH 24/35] Remove the use of Pandas' iloc() in WorldMapViz (#7379) When the same metric is used in a World Map panel for both bubble size and an axis (either X or Y), it currently breaks the rendering. The change in behavior was introduced in: https://github.com/apache/incubator-superset/commit/71e0c079041c4f955e8529349baa17058e70d256#diff-f451672348fc6071e8d627778bdc4e96L1730 The use of .iloc() is not needed anymore since the code that used to duplicate the metric is not there anymore. Should fix #7006 --- superset/viz.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/superset/viz.py b/superset/viz.py index a9e3f93e4dc85..8eb0620481941 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -1765,9 +1765,7 @@ def get_data(self, df): columns = ['country', 'm1', 'm2'] if metric == secondary_metric: ndf = df[cols] - # df[metric] will be a DataFrame - # because there are duplicate column names - ndf['m1'] = df[metric].iloc[:, 0] + ndf['m1'] = df[metric] ndf['m2'] = ndf['m1'] else: if secondary_metric: From 2a2f395e941b61a2e3d9e973689caeed81a3a6ef Mon Sep 17 00:00:00 2001 From: Ali Bahjati Date: Wed, 29 May 2019 04:37:40 +0430 Subject: [PATCH 25/35] Add "Auto" option to Mapbox visualization point radius (#7579) * Add Auto option to point radius to make it work as expected since the example using this didn't work because of this (and also as the tooltip says the auto option should be available) * Remove trailing space --- superset/assets/src/explore/controls.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index 3c239abefb5e0..feba2652562eb 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -1849,7 +1849,7 @@ export const controls = { 'Either a numerical column or `Auto`, which scales the point based ' + 'on the largest cluster'), mapStateToProps: state => ({ - choices: columnChoices(state.datasource), + choices: formatSelectOptions(['Auto']).concat(columnChoices(state.datasource)), }), }, From fd5befee38b352cf64b8d18e488751f69f97818b Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 28 May 2019 21:03:49 -0700 Subject: [PATCH 26/35] Bump python libs, address insecure releases (#7550) * Bump python libs, address insecure releases Using https://requires.io/github/mistercrunch/superset/requirements/?branch=apache_release_improv to identify insecure, old releases we're using and bumping. * redis <3.0 * fix new flakes --- requirements-dev.txt | 16 ++++++++-------- requirements.txt | 14 ++++++-------- setup.py | 7 +++---- superset/cli.py | 2 +- superset/views/core.py | 8 ++++---- tests/db_engine_specs_test.py | 2 +- tests/viz_tests.py | 8 ++++---- 7 files changed, 27 insertions(+), 30 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 4b08ec4e43a54..7059084678669 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -16,22 +16,22 @@ # coverage==4.5.3 flake8-commas==2.0.0 -flake8-import-order==0.18 +flake8-import-order==0.18.1 flake8-mypy==17.8.0 -flake8-quotes==1.0.0 -flake8==3.6.0 -flask-cors==3.0.6 -ipdb==0.11 +flake8-quotes==2.0.1 +flake8==3.7.7 +flask-cors==3.0.7 +ipdb==0.12 mypy==0.670 mysqlclient==1.4.2.post1 nose==1.3.7 -pip-tools==3.5.0 +pip-tools==3.7.0 psycopg2-binary==2.7.5 -pycodestyle==2.4.0 +pycodestyle==2.5.0 pyhive==0.6.1 pylint==1.9.2 python-dotenv==0.10.1 redis==2.10.6 statsd==3.3.0 thrift==0.11.0 -tox==3.5.3 +tox==3.11.1 diff --git a/requirements.txt b/requirements.txt index 33c6441acc6d6..3b88de6103643 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,9 +2,8 @@ # This file is autogenerated by pip-compile # To update, run: # -# pip-compile --output-file requirements.txt setup.py +# pip-compile --output-file=requirements.txt setup.py # - alembic==1.0.0 # via flask-migrate amqp==2.3.2 # via kombu apispec[yaml]==1.2.0 # via flask-appbuilder @@ -42,7 +41,7 @@ humanize==0.5.1 idna==2.6 isodate==0.6.0 itsdangerous==0.24 # via flask -jinja2==2.10 # via flask, flask-babel +jinja2==2.10.1 # via flask, flask-babel jsonschema==3.0.1 # via flask-appbuilder kombu==4.2.1 # via celery mako==1.0.7 # via alembic @@ -68,17 +67,16 @@ python-editor==1.0.3 # via alembic python-geohash==0.8.5 python3-openid==3.1.0 # via flask-openid pytz==2018.5 # via babel, celery, pandas -pyyaml==3.13 -requests==2.20.0 +pyyaml==5.1 +requests==2.22.0 retry==0.9.2 selenium==3.141.0 simplejson==3.15.0 six==1.11.0 # via bleach, cryptography, flask-jwt-extended, flask-talisman, isodate, jsonschema, pathlib2, polyline, prison, pydruid, pyrsistent, python-dateutil, sqlalchemy-utils, wtforms-json -sqlalchemy-utils==0.32.21 +sqlalchemy-utils==0.33.11 sqlalchemy==1.3.1 sqlparse==0.2.4 -unicodecsv==0.14.1 -urllib3==1.22 # via requests, selenium +urllib3==1.24.3 # via requests, selenium vine==1.1.4 # via amqp webencodings==0.5.1 # via bleach werkzeug==0.14.1 # via flask, flask-jwt-extended diff --git a/setup.py b/setup.py index 6fc74880b4784..fc91ea484d59c 100644 --- a/setup.py +++ b/setup.py @@ -97,15 +97,14 @@ def get_git_sha(): 'python-dateutil', 'python-dotenv', 'python-geohash', - 'pyyaml>=3.13', - 'requests>=2.20.0', + 'pyyaml>=5.1', + 'requests>=2.22.0', 'retry>=0.9.2', 'selenium>=3.141.0', 'simplejson>=3.15.0', 'sqlalchemy>=1.3.1,<2.0', - 'sqlalchemy-utils', + 'sqlalchemy-utils>=0.33.2', 'sqlparse', - 'unicodecsv', 'wtforms-json', ], extras_require={ diff --git a/superset/cli.py b/superset/cli.py index edb0102400571..6691b0148f8f0 100755 --- a/superset/cli.py +++ b/superset/cli.py @@ -132,7 +132,7 @@ def load_examples(load_test_data): @click.option('--datasource', '-d', help='Specify which datasource name to load, if ' 'omitted, all datasources will be refreshed') @click.option('--merge', '-m', is_flag=True, default=False, - help='Specify using \'merge\' property during operation. ' + help="Specify using 'merge' property during operation. " 'Default value is False.') def refresh_druid(datasource, merge): """Refresh druid datasources""" diff --git a/superset/views/core.py b/superset/views/core.py index 0a6ddefa18e04..d8a3692c24404 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1340,12 +1340,12 @@ def explore(self, datasource_type=None, datasource_id=None): if action == 'overwrite' and not slice_overwrite_perm: return json_error_response( - _('You don\'t have the rights to ') + _('alter this ') + _('chart'), + _("You don't have the rights to ") + _('alter this ') + _('chart'), status=400) if action == 'saveas' and not slice_add_perm: return json_error_response( - _('You don\'t have the rights to ') + _('create a ') + _('chart'), + _("You don't have the rights to ") + _('create a ') + _('chart'), status=400) if action in ('saveas', 'overwrite'): @@ -1452,7 +1452,7 @@ def save_or_overwrite_slice( dash_overwrite_perm = check_ownership(dash, raise_if_false=False) if not dash_overwrite_perm: return json_error_response( - _('You don\'t have the rights to ') + _('alter this ') + + _("You don't have the rights to ") + _('alter this ') + _('dashboard'), status=400) @@ -1466,7 +1466,7 @@ def save_or_overwrite_slice( dash_add_perm = security_manager.can_access('can_add', 'DashboardModelView') if not dash_add_perm: return json_error_response( - _('You don\'t have the rights to ') + _('create a ') + _('dashboard'), + _("You don't have the rights to ") + _('create a ') + _('dashboard'), status=400) dash = models.Dashboard( diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index e190014e1e909..0372366a2a48e 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -108,7 +108,7 @@ def test_hive_error_msg(self): '{...} errorMessage="Error while compiling statement: FAILED: ' 'SemanticException [Error 10001]: Line 4' ':5 Table not found \'fact_ridesfdslakj\'", statusCode=3, ' - 'sqlState=\'42S02\', errorCode=10001)){...}') + "sqlState='42S02', errorCode=10001)){...}") self.assertEquals(( 'Error while compiling statement: FAILED: ' 'SemanticException [Error 10001]: Line 4:5 ' diff --git a/tests/viz_tests.py b/tests/viz_tests.py index facb8c3a1525d..7c7875dbf7071 100644 --- a/tests/viz_tests.py +++ b/tests/viz_tests.py @@ -258,7 +258,7 @@ def test_parse_adhoc_filters(self): { 'expressionType': 'SQL', 'clause': 'WHERE', - 'sqlExpression': 'value3 in (\'North America\')', + 'sqlExpression': "value3 in ('North America')", }, ], } @@ -273,7 +273,7 @@ def test_parse_adhoc_filters(self): [{'op': '<', 'val': '10', 'col': 'SUM(value1)'}], query_obj['extras']['having_druid'], ) - self.assertEqual('(value3 in (\'North America\'))', query_obj['extras']['where']) + self.assertEqual("(value3 in ('North America'))", query_obj['extras']['where']) self.assertEqual('(SUM(value1) > 5)', query_obj['extras']['having']) def test_adhoc_filters_overwrite_legacy_filters(self): @@ -295,7 +295,7 @@ def test_adhoc_filters_overwrite_legacy_filters(self): { 'expressionType': 'SQL', 'clause': 'WHERE', - 'sqlExpression': 'value3 in (\'North America\')', + 'sqlExpression': "value3 in ('North America')", }, ], 'having': 'SUM(value1) > 5', @@ -311,7 +311,7 @@ def test_adhoc_filters_overwrite_legacy_filters(self): [], query_obj['extras']['having_druid'], ) - self.assertEqual('(value3 in (\'North America\'))', query_obj['extras']['where']) + self.assertEqual("(value3 in ('North America'))", query_obj['extras']['where']) self.assertEqual('', query_obj['extras']['having']) @patch('superset.viz.BaseViz.query_obj') From fc3b043462cf00e8415e8ece22f4d01b131b9cd7 Mon Sep 17 00:00:00 2001 From: "Charles S. Givre" Date: Wed, 29 May 2019 00:16:09 -0400 Subject: [PATCH 27/35] Add support for Apache Drill (#6610) * Add support for Apache Drill * Updated Docs * Removed Extraneous Functions * Removed Extraneous Functions * Final Mods * Fixed Unit Test Error * Fixed Epoch Conversion Functions --- docs/installation.rst | 31 ++++++++++++++++++++++++++ superset/db_engine_specs.py | 44 +++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/docs/installation.rst b/docs/installation.rst index 3e2934a37dff1..3405b8ab2bb95 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -392,6 +392,12 @@ Here's a list of some of the recommended packages. | Pinot | ``pip install pinotdb`` | ``pinot+http://controller:5436/`` | | | | ``query?server=http://controller:5983/`` | +---------------+-------------------------------------+-------------------------------------------------+ +| Apache Drill | | For the REST API:`` | +| | | ``drill+sadrill://`` | +| | | For JDBC | +| | | ``drill+jdbc://`` | ++---------------+-------------------------------------+-------------------------------------------------+ + Note that many other databases are supported, the main criteria being the existence of a functional SqlAlchemy dialect and Python driver. Googling @@ -449,6 +455,31 @@ Required environment variables: :: See `Teradata SQLAlchemy `_. +Apache Drill +--------- +At the time of writing, the SQLAlchemy Dialect is not available on pypi and must be downloaded here: +`SQLAlchemy Drill `_ + +Alternatively, you can install it completely from the command line as follows: :: + + git clone https://github.com/JohnOmernik/sqlalchemy-drill + cd sqlalchemy-drill + python3 setup.py install + +Once that is done, you can connect to Drill in two ways, either via the REST interface or by JDBC. If you are connecting via JDBC, you must have the +Drill JDBC Driver installed. + +The basic connection string for Drill looks like this :: + + drill+sadrill://{username}:{password}@{host}:{port}/{storage_plugin}?use_ssl=True + +If you are using JDBC to connect to Drill, the connection string looks like this: :: + + drill+jdbc://{username}:{password}@{host}:{port}/{storage_plugin} + +For a complete tutorial about how to use Apache Drill with Superset, see this tutorial: +`Visualize Anything with Superset and Drill `_ + Caching ------- diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 67aba1264b9e5..04efef78b8f37 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -724,6 +724,50 @@ def get_table_names(cls, inspector, schema): return sorted(inspector.get_table_names()) +class DrillEngineSpec(BaseEngineSpec): + """Engine spec for Apache Drill""" + engine = 'drill' + + time_grain_functions = { + None: '{col}', + 'PT1S': "nearestDate({col}, 'SECOND')", + 'PT1M': "nearestDate({col}, 'MINUTE')", + 'PT15M': "nearestDate({col}, 'QUARTER_HOUR')", + 'PT0.5H': "nearestDate({col}, 'HALF_HOUR')", + 'PT1H': "nearestDate({col}, 'HOUR')", + 'P1D': 'TO_DATE({col})', + 'P1W': "nearestDate({col}, 'WEEK_SUNDAY')", + 'P1M': "nearestDate({col}, 'MONTH')", + 'P0.25Y': "nearestDate({col}, 'QUARTER')", + 'P1Y': "nearestDate({col}, 'YEAR')", + } + + # Returns a function to convert a Unix timestamp in milliseconds to a date + @classmethod + def epoch_to_dttm(cls): + return cls.epoch_ms_to_dttm().replace('{col}', '({col}*1000)') + + @classmethod + def epoch_ms_to_dttm(cls): + return 'TO_DATE({col})' + + @classmethod + def convert_dttm(cls, target_type, dttm): + tt = target_type.upper() + if tt == 'DATE': + return "CAST('{}' AS DATE)".format(dttm.isoformat()[:10]) + elif tt == 'TIMESTAMP': + return "CAST('{}' AS TIMESTAMP)".format( + dttm.strftime('%Y-%m-%d %H:%M:%S')) + return "'{}'".format(dttm.strftime('%Y-%m-%d %H:%M:%S')) + + @classmethod + def adjust_database_uri(cls, uri, selected_schema): + if selected_schema: + uri.database = parse.quote(selected_schema, safe='') + return uri + + class MySQLEngineSpec(BaseEngineSpec): engine = 'mysql' max_column_name_length = 64 From 34407e896296a7a98a3bc31bde20ae1f3ac02005 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Thu, 30 May 2019 08:28:37 +0300 Subject: [PATCH 28/35] Make timestamp expression native SQLAlchemy element (#7131) * Add native sqla component for time expressions * Add unit tests and remove old tests * Remove redundant _grains_dict method * Clarify time_grain logic * Add docstrings and typing * Fix flake8 errors * Add missing typings * Rename to TimestampExpression * Remove redundant tests * Fix broken reference to db.database_name due to refactor --- superset/connectors/sqla/models.py | 30 +++++----- superset/db_engine_specs.py | 89 ++++++++++++++++++------------ superset/models/core.py | 10 +--- tests/db_engine_specs_test.py | 61 ++++++++++++++++++-- tests/model_tests.py | 59 -------------------- 5 files changed, 128 insertions(+), 121 deletions(-) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index f178db458bbf1..de9f4d1fd397f 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -18,6 +18,7 @@ from collections import namedtuple, OrderedDict from datetime import datetime import logging +from typing import Optional, Union from flask import escape, Markup from flask_appbuilder import Model @@ -32,11 +33,12 @@ from sqlalchemy.orm import backref, relationship from sqlalchemy.schema import UniqueConstraint from sqlalchemy.sql import column, literal_column, table, text -from sqlalchemy.sql.expression import TextAsFrom +from sqlalchemy.sql.expression import Label, TextAsFrom import sqlparse from superset import app, db, security_manager from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric +from superset.db_engine_specs import TimestampExpression from superset.jinja_context import get_template_processor from superset.models.annotations import Annotation from superset.models.core import Database @@ -140,8 +142,14 @@ def get_time_filter(self, start_dttm, end_dttm): l.append(col <= text(self.dttm_sql_literal(end_dttm, is_epoch_in_utc))) return and_(*l) - def get_timestamp_expression(self, time_grain): - """Getting the time component of the query""" + def get_timestamp_expression(self, time_grain: Optional[str]) \ + -> Union[TimestampExpression, Label]: + """ + Return a SQLAlchemy Core element representation of self to be used in a query. + + :param time_grain: Optional time grain, e.g. P1Y + :return: A TimeExpression object wrapped in a Label if supported by db + """ label = utils.DTTM_ALIAS db = self.table.database @@ -150,16 +158,12 @@ def get_timestamp_expression(self, time_grain): if not self.expression and not time_grain and not is_epoch: sqla_col = column(self.column_name, type_=DateTime) return self.table.make_sqla_column_compatible(sqla_col, label) - grain = None - if time_grain: - grain = db.grains_dict().get(time_grain) - if not grain: - raise NotImplementedError( - f'No grain spec for {time_grain} for database {db.database_name}') - col = db.db_engine_spec.get_timestamp_column(self.expression, self.column_name) - expr = db.db_engine_spec.get_time_expr(col, pdf, time_grain, grain) - sqla_col = literal_column(expr, type_=DateTime) - return self.table.make_sqla_column_compatible(sqla_col, label) + if self.expression: + col = literal_column(self.expression) + else: + col = column(self.column_name) + time_expr = db.db_engine_spec.get_timestamp_expr(col, pdf, time_grain) + return self.table.make_sqla_column_compatible(time_expr, label) @classmethod def import_obj(cls, i_column): diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 04efef78b8f37..b6103c3477600 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -36,19 +36,20 @@ import re import textwrap import time -from typing import List, Tuple +from typing import Dict, List, Optional, Tuple from urllib import parse from flask import g from flask_babel import lazy_gettext as _ import pandas import sqlalchemy as sqla -from sqlalchemy import Column, select, types +from sqlalchemy import Column, DateTime, select, types from sqlalchemy.engine import create_engine from sqlalchemy.engine.base import Engine from sqlalchemy.engine.reflection import Inspector from sqlalchemy.engine.result import RowProxy from sqlalchemy.engine.url import make_url +from sqlalchemy.ext.compiler import compiles from sqlalchemy.sql import quoted_name, text from sqlalchemy.sql.expression import ColumnClause from sqlalchemy.sql.expression import TextAsFrom @@ -90,6 +91,24 @@ } +class TimestampExpression(ColumnClause): + def __init__(self, expr: str, col: ColumnClause, **kwargs): + """Sqlalchemy class that can be can be used to render native column elements + respeting engine-specific quoting rules as part of a string-based expression. + + :param expr: Sql expression with '{col}' denoting the locations where the col + object will be rendered. + :param col: the target column + """ + super().__init__(expr, **kwargs) + self.col = col + + +@compiles(TimestampExpression) +def compile_timegrain_expression(element: TimestampExpression, compiler, **kw): + return element.name.replace('{col}', compiler.process(element.col, **kw)) + + def _create_time_grains_tuple(time_grains, time_grain_functions, blacklist): ret_list = [] blacklist = blacklist if blacklist else [] @@ -112,7 +131,7 @@ class BaseEngineSpec(object): """Abstract class for database engine specific configurations""" engine = 'base' # str as defined in sqlalchemy.engine.engine - time_grain_functions: dict = {} + time_grain_functions: Dict[Optional[str], str] = {} time_groupby_inline = False limit_method = LimitMethod.FORCE_LIMIT time_secondary_columns = False @@ -125,16 +144,31 @@ class BaseEngineSpec(object): try_remove_schema_from_table_name = True @classmethod - def get_time_expr(cls, expr, pdf, time_grain, grain): + def get_timestamp_expr(cls, col: ColumnClause, pdf: Optional[str], + time_grain: Optional[str]) -> TimestampExpression: + """ + Construct a TimeExpression to be used in a SQLAlchemy query. + + :param col: Target column for the TimeExpression + :param pdf: date format (seconds or milliseconds) + :param time_grain: time grain, e.g. P1Y for 1 year + :return: TimestampExpression object + """ + if time_grain: + time_expr = cls.time_grain_functions.get(time_grain) + if not time_expr: + raise NotImplementedError( + f'No grain spec for {time_grain} for database {cls.engine}') + else: + time_expr = '{col}' + # if epoch, translate to DATE using db specific conf if pdf == 'epoch_s': - expr = cls.epoch_to_dttm().format(col=expr) + time_expr = time_expr.replace('{col}', cls.epoch_to_dttm()) elif pdf == 'epoch_ms': - expr = cls.epoch_ms_to_dttm().format(col=expr) + time_expr = time_expr.replace('{col}', cls.epoch_ms_to_dttm()) - if grain: - expr = grain.function.format(col=expr) - return expr + return TimestampExpression(time_expr, col, type_=DateTime) @classmethod def get_time_grains(cls): @@ -489,13 +523,6 @@ def truncate_label(cls, label): label = label[:cls.max_column_name_length] return label - @staticmethod - def get_timestamp_column(expression, column_name): - """Return the expression if defined, otherwise return column_name. Some - engines require forcing quotes around column name, in which case this method - can be overridden.""" - return expression or column_name - class PostgresBaseEngineSpec(BaseEngineSpec): """ Abstract class for Postgres 'like' databases """ @@ -543,16 +570,6 @@ def get_table_names(cls, inspector, schema): tables.extend(inspector.get_foreign_table_names(schema)) return sorted(tables) - @staticmethod - def get_timestamp_column(expression, column_name): - """Postgres is unable to identify mixed case column names unless they - are quoted.""" - if expression: - return expression - elif column_name.lower() != column_name: - return f'"{column_name}"' - return column_name - class SnowflakeEngineSpec(PostgresBaseEngineSpec): engine = 'snowflake' @@ -794,7 +811,7 @@ class MySQLEngineSpec(BaseEngineSpec): 'INTERVAL DAYOFWEEK(DATE_SUB({col}, INTERVAL 1 DAY)) - 1 DAY))', } - type_code_map: dict = {} # loaded from get_datatype only if needed + type_code_map: Dict[int, str] = {} # loaded from get_datatype only if needed @classmethod def convert_dttm(cls, target_type, dttm): @@ -1812,20 +1829,21 @@ class PinotEngineSpec(BaseEngineSpec): inner_joins = False supports_column_aliases = False - _time_grain_to_datetimeconvert = { + # Pinot does its own conversion below + time_grain_functions: Dict[Optional[str], str] = { 'PT1S': '1:SECONDS', 'PT1M': '1:MINUTES', 'PT1H': '1:HOURS', 'P1D': '1:DAYS', - 'P1Y': '1:YEARS', + 'P1W': '1:WEEKS', 'P1M': '1:MONTHS', + 'P0.25Y': '3:MONTHS', + 'P1Y': '1:YEARS', } - # Pinot does its own conversion below - time_grain_functions = {k: None for k in _time_grain_to_datetimeconvert.keys()} - @classmethod - def get_time_expr(cls, expr, pdf, time_grain, grain): + def get_timestamp_expr(cls, col: ColumnClause, pdf: Optional[str], + time_grain: Optional[str]) -> TimestampExpression: is_epoch = pdf in ('epoch_s', 'epoch_ms') if not is_epoch: raise NotImplementedError('Pinot currently only supports epochs') @@ -1834,11 +1852,12 @@ def get_time_expr(cls, expr, pdf, time_grain, grain): # We are not really converting any time units, just bucketing them. seconds_or_ms = 'MILLISECONDS' if pdf == 'epoch_ms' else 'SECONDS' tf = f'1:{seconds_or_ms}:EPOCH' - granularity = cls._time_grain_to_datetimeconvert.get(time_grain) + granularity = cls.time_grain_functions.get(time_grain) if not granularity: raise NotImplementedError('No pinot grain spec for ' + str(time_grain)) # In pinot the output is a string since there is no timestamp column like pg - return f'DATETIMECONVERT({expr}, "{tf}", "{tf}", "{granularity}")' + time_expr = f'DATETIMECONVERT({{col}}, "{tf}", "{tf}", "{granularity}")' + return TimestampExpression(time_expr, col) @classmethod def make_select_compatible(cls, groupby_exprs, select_exprs): diff --git a/superset/models/core.py b/superset/models/core.py index 047a3ddb11b11..b379af7caba2b 100644 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -1029,21 +1029,13 @@ def grains(self): """Defines time granularity database-specific expressions. The idea here is to make it easy for users to change the time grain - form a datetime (maybe the source grain is arbitrary timestamps, daily + from a datetime (maybe the source grain is arbitrary timestamps, daily or 5 minutes increments) to another, "truncated" datetime. Since each database has slightly different but similar datetime functions, this allows a mapping between database engines and actual functions. """ return self.db_engine_spec.get_time_grains() - def grains_dict(self): - """Allowing to lookup grain by either label or duration - - For backward compatibility""" - d = {grain.duration: grain for grain in self.grains()} - d.update({grain.label: grain for grain in self.grains()}) - return d - def get_extra(self): extra = {} if self.extra: diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index 0372366a2a48e..43f89c1435394 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -17,15 +17,16 @@ import inspect from unittest import mock -from sqlalchemy import column, select, table -from sqlalchemy.dialects.mssql import pymssql +from sqlalchemy import column, literal_column, select, table +from sqlalchemy.dialects import mssql, oracle, postgresql from sqlalchemy.engine.result import RowProxy from sqlalchemy.types import String, UnicodeText from superset import db_engine_specs from superset.db_engine_specs import ( BaseEngineSpec, BQEngineSpec, HiveEngineSpec, MssqlEngineSpec, - MySQLEngineSpec, OracleEngineSpec, PrestoEngineSpec, + MySQLEngineSpec, OracleEngineSpec, PinotEngineSpec, PostgresEngineSpec, + PrestoEngineSpec, ) from superset.models.core import Database from .base_tests import SupersetTestCase @@ -451,7 +452,7 @@ def assert_type(type_string, type_expected): assert_type('NTEXT', UnicodeText) def test_mssql_where_clause_n_prefix(self): - dialect = pymssql.dialect() + dialect = mssql.dialect() spec = MssqlEngineSpec str_col = column('col', type_=spec.get_sqla_column_type('VARCHAR(10)')) unicode_col = column('unicode_col', type_=spec.get_sqla_column_type('NTEXT')) @@ -462,7 +463,9 @@ def test_mssql_where_clause_n_prefix(self): where(unicode_col == 'abc') query = str(sel.compile(dialect=dialect, compile_kwargs={'literal_binds': True})) - query_expected = "SELECT col, unicode_col \nFROM tbl \nWHERE col = 'abc' AND unicode_col = N'abc'" # noqa + query_expected = 'SELECT col, unicode_col \n' \ + 'FROM tbl \n' \ + "WHERE col = 'abc' AND unicode_col = N'abc'" self.assertEqual(query, query_expected) def test_get_table_names(self): @@ -483,3 +486,51 @@ def test_get_table_names(self): pg_result = db_engine_specs.PostgresEngineSpec.get_table_names( schema='schema', inspector=inspector) self.assertListEqual(pg_result_expected, pg_result) + + def test_pg_time_expression_literal_no_grain(self): + col = literal_column('COALESCE(a, b)') + expr = PostgresEngineSpec.get_timestamp_expr(col, None, None) + result = str(expr.compile(dialect=postgresql.dialect())) + self.assertEqual(result, 'COALESCE(a, b)') + + def test_pg_time_expression_literal_1y_grain(self): + col = literal_column('COALESCE(a, b)') + expr = PostgresEngineSpec.get_timestamp_expr(col, None, 'P1Y') + result = str(expr.compile(dialect=postgresql.dialect())) + self.assertEqual(result, "DATE_TRUNC('year', COALESCE(a, b))") + + def test_pg_time_expression_lower_column_no_grain(self): + col = column('lower_case') + expr = PostgresEngineSpec.get_timestamp_expr(col, None, None) + result = str(expr.compile(dialect=postgresql.dialect())) + self.assertEqual(result, 'lower_case') + + def test_pg_time_expression_lower_case_column_sec_1y_grain(self): + col = column('lower_case') + expr = PostgresEngineSpec.get_timestamp_expr(col, 'epoch_s', 'P1Y') + result = str(expr.compile(dialect=postgresql.dialect())) + self.assertEqual(result, "DATE_TRUNC('year', (timestamp 'epoch' + lower_case * interval '1 second'))") # noqa + + def test_pg_time_expression_mixed_case_column_1y_grain(self): + col = column('MixedCase') + expr = PostgresEngineSpec.get_timestamp_expr(col, None, 'P1Y') + result = str(expr.compile(dialect=postgresql.dialect())) + self.assertEqual(result, "DATE_TRUNC('year', \"MixedCase\")") + + def test_mssql_time_expression_mixed_case_column_1y_grain(self): + col = column('MixedCase') + expr = MssqlEngineSpec.get_timestamp_expr(col, None, 'P1Y') + result = str(expr.compile(dialect=mssql.dialect())) + self.assertEqual(result, 'DATEADD(year, DATEDIFF(year, 0, [MixedCase]), 0)') + + def test_oracle_time_expression_reserved_keyword_1m_grain(self): + col = column('decimal') + expr = OracleEngineSpec.get_timestamp_expr(col, None, 'P1M') + result = str(expr.compile(dialect=oracle.dialect())) + self.assertEqual(result, "TRUNC(CAST(\"decimal\" as DATE), 'MONTH')") + + def test_pinot_time_expression_sec_1m_grain(self): + col = column('tstamp') + expr = PinotEngineSpec.get_timestamp_expr(col, 'epoch_s', 'P1M') + result = str(expr.compile()) + self.assertEqual(result, 'DATETIMECONVERT(tstamp, "1:SECONDS:EPOCH", "1:SECONDS:EPOCH", "1:MONTHS")') # noqa diff --git a/tests/model_tests.py b/tests/model_tests.py index 0fe03de932a28..53e53cc5f6516 100644 --- a/tests/model_tests.py +++ b/tests/model_tests.py @@ -109,47 +109,6 @@ def test_select_star(self): LIMIT 100""") assert sql.startswith(expected) - def test_grains_dict(self): - uri = 'mysql://root@localhost' - database = Database(sqlalchemy_uri=uri) - d = database.grains_dict() - self.assertEquals(d.get('day').function, 'DATE({col})') - self.assertEquals(d.get('P1D').function, 'DATE({col})') - self.assertEquals(d.get('Time Column').function, '{col}') - - def test_postgres_expression_time_grain(self): - uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset' - database = Database(sqlalchemy_uri=uri) - pdf, time_grain = '', 'P1D' - expression, column_name = 'COALESCE(lowercase_col, "MixedCaseCol")', '' - grain = database.grains_dict().get(time_grain) - col = database.db_engine_spec.get_timestamp_column(expression, column_name) - grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain) - grain_expr_expected = grain.function.replace('{col}', expression) - self.assertEqual(grain_expr, grain_expr_expected) - - def test_postgres_lowercase_col_time_grain(self): - uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset' - database = Database(sqlalchemy_uri=uri) - pdf, time_grain = '', 'P1D' - expression, column_name = '', 'lowercase_col' - grain = database.grains_dict().get(time_grain) - col = database.db_engine_spec.get_timestamp_column(expression, column_name) - grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain) - grain_expr_expected = grain.function.replace('{col}', column_name) - self.assertEqual(grain_expr, grain_expr_expected) - - def test_postgres_mixedcase_col_time_grain(self): - uri = 'postgresql+psycopg2://uid:pwd@localhost:5432/superset' - database = Database(sqlalchemy_uri=uri) - pdf, time_grain = '', 'P1D' - expression, column_name = '', 'MixedCaseCol' - grain = database.grains_dict().get(time_grain) - col = database.db_engine_spec.get_timestamp_column(expression, column_name) - grain_expr = database.db_engine_spec.get_time_expr(col, pdf, time_grain, grain) - grain_expr_expected = grain.function.replace('{col}', f'"{column_name}"') - self.assertEqual(grain_expr, grain_expr_expected) - def test_single_statement(self): main_db = get_main_database(db.session) @@ -217,24 +176,6 @@ def test_get_timestamp_expression_epoch(self): self.assertEquals(compiled, 'DATE(from_unixtime(DATE_ADD(ds, 1)))') ds_col.expression = prev_ds_expr - def test_get_timestamp_expression_backward(self): - tbl = self.get_table_by_name('birth_names') - ds_col = tbl.get_column('ds') - - ds_col.expression = None - ds_col.python_date_format = None - sqla_literal = ds_col.get_timestamp_expression('day') - compiled = '{}'.format(sqla_literal.compile()) - if tbl.database.backend == 'mysql': - self.assertEquals(compiled, 'DATE(ds)') - - ds_col.expression = None - ds_col.python_date_format = None - sqla_literal = ds_col.get_timestamp_expression('Time Column') - compiled = '{}'.format(sqla_literal.compile()) - if tbl.database.backend == 'mysql': - self.assertEquals(compiled, 'ds') - def query_with_expr_helper(self, is_timeseries, inner_join=True): tbl = self.get_table_by_name('birth_names') ds_col = tbl.get_column('ds') From 145d72c52b0dbb44e1d77a2562cfe68fc9a7b3b7 Mon Sep 17 00:00:00 2001 From: Erik Ritter Date: Thu, 30 May 2019 10:37:24 -0700 Subject: [PATCH 29/35] Fix SQL Lab window resizing layout bug (#7615) --- .../javascripts/sqllab/SqlEditor_spec.jsx | 38 +++++++++++++- .../src/SqlLab/components/SqlEditor.jsx | 50 +++++++++++++------ .../SqlLab/components/SqlEditorLeftBar.jsx | 3 +- superset/assets/src/SqlLab/constants.js | 5 ++ 4 files changed, 80 insertions(+), 16 deletions(-) diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx index 046b2e69cf89f..17bb4d83be2ad 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -20,10 +20,19 @@ import React from 'react'; import { shallow } from 'enzyme'; import { defaultQueryEditor, initialState, queries, table } from './fixtures'; +import { + SQL_EDITOR_GUTTER_HEIGHT, + SQL_EDITOR_GUTTER_MARGIN, + SQL_TOOLBAR_HEIGHT, +} from '../../../src/SqlLab/constants'; +import AceEditorWrapper from '../../../src/SqlLab/components/AceEditorWrapper'; import LimitControl from '../../../src/SqlLab/components/LimitControl'; +import SouthPane from '../../../src/SqlLab/components/SouthPane'; import SqlEditor from '../../../src/SqlLab/components/SqlEditor'; import SqlEditorLeftBar from '../../../src/SqlLab/components/SqlEditorLeftBar'; +const MOCKED_SQL_EDITOR_HEIGHT = 500; + describe('SqlEditor', () => { const mockedProps = { actions: {}, @@ -40,7 +49,7 @@ describe('SqlEditor', () => { }; beforeAll(() => { - jest.spyOn(SqlEditor.prototype, 'getSqlEditorHeight').mockImplementation(() => 500); + jest.spyOn(SqlEditor.prototype, 'getSqlEditorHeight').mockImplementation(() => MOCKED_SQL_EDITOR_HEIGHT); }); it('is valid', () => { @@ -52,6 +61,33 @@ describe('SqlEditor', () => { const wrapper = shallow(); expect(wrapper.find(SqlEditorLeftBar)).toHaveLength(1); }); + it('render an AceEditorWrapper', () => { + const wrapper = shallow(); + expect(wrapper.find(AceEditorWrapper)).toHaveLength(1); + }); + it('render an SouthPane', () => { + const wrapper = shallow(); + expect(wrapper.find(SouthPane)).toHaveLength(1); + }); + it('does not overflow the editor window', () => { + const wrapper = shallow(); + const totalSize = parseFloat(wrapper.find(AceEditorWrapper).props().height) + + wrapper.find(SouthPane).props().height + + SQL_TOOLBAR_HEIGHT + + (SQL_EDITOR_GUTTER_MARGIN * 2) + + SQL_EDITOR_GUTTER_HEIGHT; + expect(totalSize).toEqual(MOCKED_SQL_EDITOR_HEIGHT); + }); + it('does not overflow the editor window after resizing', () => { + const wrapper = shallow(); + wrapper.setState({ height: 450 }); + const totalSize = parseFloat(wrapper.find(AceEditorWrapper).props().height) + + wrapper.find(SouthPane).props().height + + SQL_TOOLBAR_HEIGHT + + (SQL_EDITOR_GUTTER_MARGIN * 2) + + SQL_EDITOR_GUTTER_HEIGHT; + expect(totalSize).toEqual(450); + }); it('render a LimitControl with default limit', () => { const defaultQueryLimit = 101; const updatedProps = { ...mockedProps, defaultQueryLimit }; diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx index a4aabb73fd49a..84e51064595dd 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -31,6 +31,7 @@ import { import Split from 'react-split'; import { t } from '@superset-ui/translation'; import debounce from 'lodash/debounce'; +import throttle from 'lodash/throttle'; import Button from '../../components/Button'; import LimitControl from './LimitControl'; @@ -43,17 +44,20 @@ import Timer from '../../components/Timer'; import Hotkeys from '../../components/Hotkeys'; import SqlEditorLeftBar from './SqlEditorLeftBar'; import AceEditorWrapper from './AceEditorWrapper'; -import { STATE_BSSTYLE_MAP } from '../constants'; +import { + STATE_BSSTYLE_MAP, + SQL_EDITOR_GUTTER_HEIGHT, + SQL_EDITOR_GUTTER_MARGIN, + SQL_TOOLBAR_HEIGHT, +} from '../constants'; import RunQueryActionButton from './RunQueryActionButton'; import { FeatureFlag, isFeatureEnabled } from '../../featureFlags'; const SQL_EDITOR_PADDING = 10; -const SQL_TOOLBAR_HEIGHT = 51; -const GUTTER_HEIGHT = 5; -const GUTTER_MARGIN = 3; const INITIAL_NORTH_PERCENT = 30; const INITIAL_SOUTH_PERCENT = 70; const VALIDATION_DEBOUNCE_MS = 600; +const WINDOW_RESIZE_THROTTLE_MS = 100; const propTypes = { actions: PropTypes.object.isRequired, @@ -83,6 +87,8 @@ class SqlEditor extends React.PureComponent { this.state = { autorun: props.queryEditor.autorun, ctas: '', + northPercent: INITIAL_NORTH_PERCENT, + southPercent: INITIAL_SOUTH_PERCENT, sql: props.queryEditor.sql, }; this.sqlEditorRef = React.createRef(); @@ -103,6 +109,10 @@ class SqlEditor extends React.PureComponent { this.requestValidation.bind(this), VALIDATION_DEBOUNCE_MS, ); + this.handleWindowResize = throttle( + this.handleWindowResize.bind(this), + WINDOW_RESIZE_THROTTLE_MS, + ); } componentWillMount() { if (this.state.autorun) { @@ -116,6 +126,11 @@ class SqlEditor extends React.PureComponent { // the south pane so it gets rendered properly // eslint-disable-next-line react/no-did-mount-set-state this.setState({ height: this.getSqlEditorHeight() }); + + window.addEventListener('resize', this.handleWindowResize); + } + componentWillUnmount() { + window.removeEventListener('resize', this.handleWindowResize); } onResizeStart() { // Set the heights on the ace editor and the ace content area after drag starts @@ -124,8 +139,7 @@ class SqlEditor extends React.PureComponent { document.getElementsByClassName('ace_content')[0].style.height = '100%'; } onResizeEnd([northPercent, southPercent]) { - this.setState(this.getAceEditorAndSouthPaneHeights( - this.state.height, northPercent, southPercent)); + this.setState({ northPercent, southPercent }); if (this.northPaneRef.current && this.northPaneRef.current.clientHeight) { this.props.actions.persistEditorHeight(this.props.queryEditor, @@ -149,9 +163,11 @@ class SqlEditor extends React.PureComponent { // given the height of the sql editor, north pane percent and south pane percent. getAceEditorAndSouthPaneHeights(height, northPercent, southPercent) { return { - aceEditorHeight: height * northPercent / 100 - (GUTTER_HEIGHT / 2 + GUTTER_MARGIN) + aceEditorHeight: height * northPercent / 100 + - (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN) - SQL_TOOLBAR_HEIGHT, - southPaneHeight: height * southPercent / 100 - (GUTTER_HEIGHT / 2 + GUTTER_MARGIN), + southPaneHeight: height * southPercent / 100 + - (SQL_EDITOR_GUTTER_HEIGHT / 2 + SQL_EDITOR_GUTTER_MARGIN), }; } getHotkeyConfig() { @@ -194,9 +210,12 @@ class SqlEditor extends React.PureComponent { setQueryLimit(queryLimit) { this.props.actions.queryEditorSetQueryLimit(this.props.queryEditor, queryLimit); } + handleWindowResize() { + this.setState({ height: this.getSqlEditorHeight() }); + } elementStyle(dimension, elementSize, gutterSize) { return { - [dimension]: `calc(${elementSize}% - ${gutterSize + GUTTER_MARGIN}px)`, + [dimension]: `calc(${elementSize}% - ${gutterSize + SQL_EDITOR_GUTTER_MARGIN}px)`, }; } requestValidation() { @@ -257,15 +276,18 @@ class SqlEditor extends React.PureComponent { queryPane() { const hotkeys = this.getHotkeyConfig(); const { aceEditorHeight, southPaneHeight } = this.getAceEditorAndSouthPaneHeights( - this.state.height, INITIAL_NORTH_PERCENT, INITIAL_SOUTH_PERCENT); + this.state.height, + this.state.northPercent, + this.state.southPercent, + ); return ( @@ -277,7 +299,7 @@ class SqlEditor extends React.PureComponent { queryEditor={this.props.queryEditor} sql={this.props.queryEditor.sql} tables={this.props.tables} - height={`${this.state.aceEditorHeight || aceEditorHeight}px`} + height={`${aceEditorHeight}px`} hotkeys={hotkeys} /> {this.renderEditorBottomBar(hotkeys)} @@ -286,7 +308,7 @@ class SqlEditor extends React.PureComponent { editorQueries={this.props.editorQueries} dataPreviewQueries={this.props.dataPreviewQueries} actions={this.props.actions} - height={this.state.southPaneHeight || southPaneHeight} + height={southPaneHeight} /> ); diff --git a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx index 43ea4873a0f73..f389641117467 100644 --- a/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditorLeftBar.jsx @@ -33,9 +33,10 @@ const propTypes = { }; const defaultProps = { - tables: [], actions: {}, + height: 500, offline: false, + tables: [], }; export default class SqlEditorLeftBar extends React.PureComponent { diff --git a/superset/assets/src/SqlLab/constants.js b/superset/assets/src/SqlLab/constants.js index 3bf8ce0bf3fba..4dd2118d7901e 100644 --- a/superset/assets/src/SqlLab/constants.js +++ b/superset/assets/src/SqlLab/constants.js @@ -43,3 +43,8 @@ export const TIME_OPTIONS = [ '90 days ago', '1 year ago', ]; + +// SqlEditor layout constants +export const SQL_EDITOR_GUTTER_HEIGHT = 5; +export const SQL_EDITOR_GUTTER_MARGIN = 3; +export const SQL_TOOLBAR_HEIGHT = 51; From dbdb6b093aeb0b482494e7a4ee65b5836b5070ea Mon Sep 17 00:00:00 2001 From: Grace Guo Date: Thu, 30 May 2019 10:38:49 -0700 Subject: [PATCH 30/35] [SQL Lab] fix unnecessary offline action (#7594) --- .../sqllab/QueryAutoRefresh_spec.jsx | 72 +++++++++++++++++++ .../spec/javascripts/sqllab/fixtures.js | 2 + .../SqlLab/components/QueryAutoRefresh.jsx | 23 ++++-- 3 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx diff --git a/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx b/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx new file mode 100644 index 0000000000000..527ecd680980b --- /dev/null +++ b/superset/assets/spec/javascripts/sqllab/QueryAutoRefresh_spec.jsx @@ -0,0 +1,72 @@ +/** + * 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. + */ +import React from 'react'; +import { shallow } from 'enzyme'; +import sinon from 'sinon'; +import thunk from 'redux-thunk'; +import configureStore from 'redux-mock-store'; + +import QueryAutoRefresh from '../../../src/SqlLab/components/QueryAutoRefresh'; +import { initialState, runningQuery } from './fixtures'; + +describe('QueryAutoRefresh', () => { + const middlewares = [thunk]; + const mockStore = configureStore(middlewares); + const sqlLab = { + ...initialState.sqlLab, + queries: { + ryhMUZCGb: runningQuery, + }, + }; + const state = { + ...initialState, + sqlLab, + + }; + const store = mockStore(state); + + const getWrapper = () => ( + shallow(, { + context: { store }, + }).dive()); + + let wrapper; + + it('shouldCheckForQueries', () => { + wrapper = getWrapper(); + expect(wrapper.instance().shouldCheckForQueries()).toBe(true); + }); + + it('setUserOffline', () => { + wrapper = getWrapper(); + const spy = sinon.spy(wrapper.instance().props.actions, 'setUserOffline'); + + // state not changed + wrapper.setState({ + offline: false, + }); + expect(spy.called).toBe(false); + + // state is changed + wrapper.setState({ + offline: true, + }); + expect(spy.callCount).toBe(1); + }); +}); diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index f43f43f550a68..6bd090ecaf841 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -366,11 +366,13 @@ export const runningQuery = { id: 'ryhMUZCGb', progress: 90, state: 'running', + startDttm: Date.now() - 500, }; export const cachedQuery = Object.assign({}, queries[0], { cached: true }); export const initialState = { sqlLab: { + offline: false, alerts: [], queries: {}, databases: {}, diff --git a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx index 6704d393fe754..3fcab31a64de0 100644 --- a/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx +++ b/superset/assets/src/SqlLab/components/QueryAutoRefresh.jsx @@ -30,9 +30,20 @@ const MAX_QUERY_AGE_TO_POLL = 21600000; const QUERY_TIMEOUT_LIMIT = 10000; class QueryAutoRefresh extends React.PureComponent { + constructor(props) { + super(props); + this.state = { + offline: props.offline, + }; + } componentWillMount() { this.startTimer(); } + componentDidUpdate(prevProps) { + if (prevProps.offline !== this.state.offline) { + this.props.actions.setUserOffline(this.state.offline); + } + } componentWillUnmount() { this.stopTimer(); } @@ -70,12 +81,12 @@ class QueryAutoRefresh extends React.PureComponent { if (Object.keys(json).length > 0) { this.props.actions.refreshQueries(json); } - this.props.actions.setUserOffline(false); - }).catch(() => { - this.props.actions.setUserOffline(true); - }); + this.setState({ offline: false }); + }).catch(() => { + this.setState({ offline: true }); + }); } else { - this.props.actions.setUserOffline(false); + this.setState({ offline: false }); } } render() { @@ -83,6 +94,7 @@ class QueryAutoRefresh extends React.PureComponent { } } QueryAutoRefresh.propTypes = { + offline: PropTypes.bool.isRequired, queries: PropTypes.object.isRequired, actions: PropTypes.object.isRequired, queriesLastUpdate: PropTypes.number.isRequired, @@ -90,6 +102,7 @@ QueryAutoRefresh.propTypes = { function mapStateToProps({ sqlLab }) { return { + offline: sqlLab.offline, queries: sqlLab.queries, queriesLastUpdate: sqlLab.queriesLastUpdate, }; From c1712e5d1080c2ca4d80cf581256fd1b9e23e59c Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Fri, 31 May 2019 16:55:26 +0300 Subject: [PATCH 31/35] Add relative start param for time filters (#7525) * Add relative start param for time filters * Fix typo and add types to parse_human_datetime * Add relative start/end to query_object * Fix linting error --- superset/common/query_object.py | 9 ++++++++- superset/config.py | 7 ++++++- superset/utils/core.py | 22 ++++++++++++---------- superset/viz.py | 7 +++++-- tests/utils_tests.py | 22 +++++++++++++++++++--- 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/superset/common/query_object.py b/superset/common/query_object.py index 47abbf2a091bd..553c0b9ddcfd7 100644 --- a/superset/common/query_object.py +++ b/superset/common/query_object.py @@ -51,9 +51,16 @@ def __init__( is_prequery: bool = False, columns: List[str] = None, orderby: List[List] = None, + relative_start: str = app.config.get('DEFAULT_RELATIVE_START_TIME', 'today'), + relative_end: str = app.config.get('DEFAULT_RELATIVE_END_TIME', 'today'), ): self.granularity = granularity - self.from_dttm, self.to_dttm = utils.get_since_until(time_range, time_shift) + self.from_dttm, self.to_dttm = utils.get_since_until( + relative_start=relative_start, + relative_end=relative_end, + time_range=time_range, + time_shift=time_shift, + ) self.is_timeseries = is_timeseries self.time_range = time_range self.time_shift = utils.parse_human_timedelta(time_shift) diff --git a/superset/config.py b/superset/config.py index fc287b7f2c13d..7d6ca33e5f9aa 100644 --- a/superset/config.py +++ b/superset/config.py @@ -599,8 +599,13 @@ class CeleryConfig(object): DOCUMENTATION_URL = None # What is the Last N days relative in the time selector to: -# 'today' means it is midnight (00:00:00) of today in the local timezone +# 'today' means it is midnight (00:00:00) in the local timezone # 'now' means it is relative to the query issue time +# If both start and end time is set to now, this will make the time +# filter a moving window. By only setting the end time to now, +# start time will be set to midnight, while end will be relative to +# the query issue time. +DEFAULT_RELATIVE_START_TIME = 'today' DEFAULT_RELATIVE_END_TIME = 'today' # Is epoch_s/epoch_ms datetime format supposed to be considered since UTC ? diff --git a/superset/utils/core.py b/superset/utils/core.py index 2defa70dd179e..df2550040d947 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -237,14 +237,14 @@ def parse_human_datetime(s): # when time is not extracted, we 'reset to midnight' if parsed_flags & 2 == 0: parsed_dttm = parsed_dttm.replace(hour=0, minute=0, second=0) - dttm = dttm_from_timtuple(parsed_dttm.utctimetuple()) + dttm = dttm_from_timetuple(parsed_dttm.utctimetuple()) except Exception as e: logging.exception(e) raise ValueError("Couldn't parse date string [{}]".format(s)) return dttm -def dttm_from_timtuple(d: struct_time) -> datetime: +def dttm_from_timetuple(d: struct_time) -> datetime: return datetime( d.tm_year, d.tm_mon, d.tm_mday, d.tm_hour, d.tm_min, d.tm_sec) @@ -306,7 +306,7 @@ def parse_human_timedelta(s: str): True """ cal = parsedatetime.Calendar() - dttm = dttm_from_timtuple(datetime.now().timetuple()) + dttm = dttm_from_timetuple(datetime.now().timetuple()) d = cal.parse(s or '', dttm)[0] d = datetime(d.tm_year, d.tm_mon, d.tm_mday, d.tm_hour, d.tm_min, d.tm_sec) return d - dttm @@ -939,6 +939,7 @@ def get_since_until(time_range: Optional[str] = None, since: Optional[str] = None, until: Optional[str] = None, time_shift: Optional[str] = None, + relative_start: Optional[str] = None, relative_end: Optional[str] = None) -> Tuple[datetime, datetime]: """Return `since` and `until` date time tuple from string representations of time_range, since, until and time_shift. @@ -965,13 +966,14 @@ def get_since_until(time_range: Optional[str] = None, """ separator = ' : ' + relative_start = parse_human_datetime(relative_start if relative_start else 'today') relative_end = parse_human_datetime(relative_end if relative_end else 'today') common_time_frames = { - 'Last day': (relative_end - relativedelta(days=1), relative_end), # noqa: T400 - 'Last week': (relative_end - relativedelta(weeks=1), relative_end), # noqa: T400 - 'Last month': (relative_end - relativedelta(months=1), relative_end), # noqa: E501, T400 - 'Last quarter': (relative_end - relativedelta(months=3), relative_end), # noqa: E501, T400 - 'Last year': (relative_end - relativedelta(years=1), relative_end), # noqa: T400 + 'Last day': (relative_start - relativedelta(days=1), relative_end), # noqa: T400 + 'Last week': (relative_start - relativedelta(weeks=1), relative_end), # noqa: E501, T400 + 'Last month': (relative_start - relativedelta(months=1), relative_end), # noqa: E501, T400 + 'Last quarter': (relative_start - relativedelta(months=3), relative_end), # noqa: E501, T400 + 'Last year': (relative_start - relativedelta(years=1), relative_end), # noqa: E501, T400 } if time_range: @@ -988,10 +990,10 @@ def get_since_until(time_range: Optional[str] = None, else: rel, num, grain = time_range.split() if rel == 'Last': - since = relative_end - relativedelta(**{grain: int(num)}) # noqa: T400 + since = relative_start - relativedelta(**{grain: int(num)}) # noqa: T400 until = relative_end else: # rel == 'Next' - since = relative_end + since = relative_start until = relative_end + relativedelta(**{grain: int(num)}) # noqa: T400 else: since = since or '' diff --git a/superset/viz.py b/superset/viz.py index 8eb0620481941..a6864d32312cc 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -59,6 +59,7 @@ config = app.config stats_logger = config.get('STATS_LOGGER') +relative_start = config.get('DEFAULT_RELATIVE_START_TIME', 'today') relative_end = config.get('DEFAULT_RELATIVE_END_TIME', 'today') METRIC_KEYS = [ @@ -274,7 +275,8 @@ def query_obj(self): # default order direction order_desc = form_data.get('order_desc', True) - since, until = utils.get_since_until(relative_end=relative_end, + since, until = utils.get_since_until(relative_start=relative_start, + relative_end=relative_end, time_range=form_data.get('time_range'), since=form_data.get('since'), until=form_data.get('until')) @@ -800,7 +802,8 @@ def get_data(self, df): values[str(v / 10**9)] = obj.get(metric) data[metric] = values - start, end = utils.get_since_until(relative_end=relative_end, + start, end = utils.get_since_until(relative_start=relative_start, + relative_end=relative_end, time_range=form_data.get('time_range'), since=form_data.get('since'), until=form_data.get('until')) diff --git a/tests/utils_tests.py b/tests/utils_tests.py index 40dedafaef597..a39631b832274 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -43,7 +43,9 @@ def mock_parse_human_datetime(s): - if s in ['now', 'today']: + if s == 'now': + return datetime(2016, 11, 7, 9, 30, 10) + elif s == 'today': return datetime(2016, 11, 7) elif s == 'yesterday': return datetime(2016, 11, 6) @@ -51,6 +53,8 @@ def mock_parse_human_datetime(s): return datetime(2016, 11, 8) elif s == 'Last year': return datetime(2015, 11, 7) + elif s == 'Last week': + return datetime(2015, 10, 31) elif s == 'Last 5 months': return datetime(2016, 6, 7) elif s == 'Next 5 months': @@ -600,7 +604,7 @@ def test_get_since_until(self): self.assertEqual(result, expected) result = get_since_until(' : now') - expected = None, datetime(2016, 11, 7) + expected = None, datetime(2016, 11, 7, 9, 30, 10) self.assertEqual(result, expected) result = get_since_until('yesterday : tomorrow') @@ -636,7 +640,19 @@ def test_get_since_until(self): self.assertEqual(result, expected) result = get_since_until(time_range='5 days : now') - expected = datetime(2016, 11, 2), datetime(2016, 11, 7) + expected = datetime(2016, 11, 2), datetime(2016, 11, 7, 9, 30, 10) + self.assertEqual(result, expected) + + result = get_since_until('Last week', relative_end='now') + expected = datetime(2016, 10, 31), datetime(2016, 11, 7, 9, 30, 10) + self.assertEqual(result, expected) + + result = get_since_until('Last week', relative_start='now') + expected = datetime(2016, 10, 31, 9, 30, 10), datetime(2016, 11, 7) + self.assertEqual(result, expected) + + result = get_since_until('Last week', relative_start='now', relative_end='now') + expected = datetime(2016, 10, 31, 9, 30, 10), datetime(2016, 11, 7, 9, 30, 10) self.assertEqual(result, expected) with self.assertRaises(ValueError): From d408e30594327ef311e4b7470721edbc72bcc508 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Fri, 31 May 2019 10:17:13 -0700 Subject: [PATCH 32/35] Show expanded columns in gray in SQL Editor (#7627) * Show expanded columns in gray * Remove payload mocking * Remove empty line * Safety fallback if expanded_columns is not in the payload * Fix typo --- superset/assets/src/SqlLab/components/ResultSet.jsx | 4 ++++ .../components/FilterableTable/FilterableTable.jsx | 12 +++++++++++- .../FilterableTable/FilterableTableStyles.css | 5 ++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/SqlLab/components/ResultSet.jsx b/superset/assets/src/SqlLab/components/ResultSet.jsx index 443eb16b29998..7ac04b3292c51 100644 --- a/superset/assets/src/SqlLab/components/ResultSet.jsx +++ b/superset/assets/src/SqlLab/components/ResultSet.jsx @@ -207,6 +207,9 @@ export default class ResultSet extends React.PureComponent { data = results.data; } if (data && data.length > 0) { + const expandedColumns = results.expanded_columns + ? results.expanded_columns.map(col => col.name) + : []; return ( {this.renderControls.bind(this)()} @@ -216,6 +219,7 @@ export default class ResultSet extends React.PureComponent { orderedColumnKeys={results.columns.map(col => col.name)} height={height} filterText={this.state.searchText} + expandedColumns={expandedColumns} /> ); diff --git a/superset/assets/src/components/FilterableTable/FilterableTable.jsx b/superset/assets/src/components/FilterableTable/FilterableTable.jsx index 702f2b96493f8..e4f21a514d5e6 100644 --- a/superset/assets/src/components/FilterableTable/FilterableTable.jsx +++ b/superset/assets/src/components/FilterableTable/FilterableTable.jsx @@ -44,6 +44,7 @@ const propTypes = { overscanRowCount: PropTypes.number, rowHeight: PropTypes.number, striped: PropTypes.bool, + expandedColumns: PropTypes.array, }; const defaultProps = { @@ -52,6 +53,7 @@ const defaultProps = { overscanRowCount: 10, rowHeight: 32, striped: true, + expandedColumns: [], }; export default class FilterableTable extends PureComponent { @@ -141,7 +143,15 @@ export default class FilterableTable extends PureComponent { return (
- {label} + -1 + ? 'header-style-disabled' + : '' + } + > + {label} + {sortBy === dataKey && } diff --git a/superset/assets/src/components/FilterableTable/FilterableTableStyles.css b/superset/assets/src/components/FilterableTable/FilterableTableStyles.css index 7a0d3ba0ea7d3..f24df737e9bc2 100644 --- a/superset/assets/src/components/FilterableTable/FilterableTableStyles.css +++ b/superset/assets/src/components/FilterableTable/FilterableTableStyles.css @@ -76,4 +76,7 @@ overflow: hidden; text-overflow: ellipsis; white-space: nowrap; -} \ No newline at end of file +} +.header-style-disabled { + color: #aaa; +} From d2967340d9e73eb16a590209acd8dd73ef0e1af9 Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Fri, 31 May 2019 11:25:07 -0700 Subject: [PATCH 33/35] View Presto row and array objects clearly in the data grid (#7625) * feat: rough check in for Presto rows and arrays * fix: presto arrays * fix: return selected and expanded columns * fix: add helper methods and unit tests * fix: only allow exploration of selected columns * fix: address Beto's comments and add more unit tests --- .../spec/javascripts/sqllab/fixtures.js | 16 +- .../components/ExploreResultsButton.jsx | 6 +- superset/assets/src/SqlLab/main.less | 1 + superset/db_engine_specs.py | 436 ++++++++++++++++-- superset/sql_lab.py | 11 +- tests/db_engine_specs_test.py | 281 ++++++++++- 6 files changed, 709 insertions(+), 42 deletions(-) diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index 6bd090ecaf841..99e740c338237 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -223,6 +223,20 @@ export const queries = [ type: 'STRING', }, ], + selected_columns: [ + { + is_date: true, + is_dim: false, + name: 'ds', + type: 'STRING', + }, + { + is_date: false, + is_dim: true, + name: 'gender', + type: 'STRING', + }, + ], data: [{ col1: 0, col2: 1 }, { col1: 2, col2: 3 }], }, }, @@ -264,7 +278,7 @@ export const queryWithBadColumns = { ...queries[0], results: { data: queries[0].results.data, - columns: [ + selected_columns: [ { is_date: true, is_dim: false, diff --git a/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx b/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx index 2394c6713a942..5ac1e2570df1a 100644 --- a/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx +++ b/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx @@ -85,8 +85,8 @@ class ExploreResultsButton extends React.PureComponent { } getColumns() { const props = this.props; - if (props.query && props.query.results && props.query.results.columns) { - return props.query.results.columns; + if (props.query && props.query.results && props.query.results.selected_columns) { + return props.query.results.selected_columns; } return []; } @@ -97,7 +97,7 @@ class ExploreResultsButton extends React.PureComponent { const re1 = /^[A-Za-z_]\w*$/; // starts with char or _, then only alphanum const re2 = /__\d+$/; // does not finish with __ and then a number which screams dup col name - return this.props.query.results.columns.map(col => col.name) + return this.props.query.results.selected_columns.map(col => col.name) .filter(col => !re1.test(col) || re2.test(col)); } datasourceName() { diff --git a/superset/assets/src/SqlLab/main.less b/superset/assets/src/SqlLab/main.less index 822e7d84ce4dc..c7be8fbc20c6a 100644 --- a/superset/assets/src/SqlLab/main.less +++ b/superset/assets/src/SqlLab/main.less @@ -238,6 +238,7 @@ div.Workspace { .schemaPane { flex: 0 0 300px; + max-width: 300px; transition: all .3s ease-in-out; } diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index b6103c3477600..00a9310b2acf4 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -28,7 +28,7 @@ The general idea is to use static classes and an inheritance scheme. """ -from collections import namedtuple +from collections import namedtuple, OrderedDict import hashlib import inspect import logging @@ -36,7 +36,7 @@ import re import textwrap import time -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Set, Tuple from urllib import parse from flask import g @@ -194,6 +194,12 @@ def fetch_data(cls, cursor, limit): return cursor.fetchmany(limit) return cursor.fetchall() + @classmethod + def expand_data(cls, + columns: List[dict], + data: List[dict]) -> Tuple[List[dict], List[dict], List[dict]]: + return columns, data, [] + @classmethod def alter_new_orm_column(cls, orm_col): """Allow altering default column attributes when first detected/added @@ -891,20 +897,16 @@ def get_view_names(cls, inspector, schema): return [] @classmethod - def _create_column_info(cls, column: RowProxy, name: str, data_type: str) -> dict: + def _create_column_info(cls, name: str, data_type: str) -> dict: """ Create column info object - :param column: column object :param name: column name :param data_type: column data type :return: column info object """ return { 'name': name, - 'type': data_type, - # newer Presto no longer includes this column - 'nullable': getattr(column, 'Null', True), - 'default': None, + 'type': f'{data_type}', } @classmethod @@ -943,13 +945,11 @@ def _split_data_type(cls, data_type: str, delimiter: str) -> List[str]: r'{}(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)'.format(delimiter), data_type) @classmethod - def _parse_structural_column(cls, column: RowProxy, result: List[dict]) -> None: + def _parse_structural_column(cls, full_data_type: str, result: List[dict]) -> None: """ Parse a row or array column - :param column: column :param result: list tracking the results """ - full_data_type = '{} {}'.format(column.Column, column.Type) # split on open parenthesis ( to get the structural # data type and its component types data_types = cls._split_data_type(full_data_type, r'\(') @@ -964,8 +964,9 @@ def _parse_structural_column(cls, column: RowProxy, result: List[dict]) -> None: stack.pop() elif cls._has_nested_data_types(inner_type): # split on comma , to get individual data types - single_fields = cls._split_data_type(inner_type, ', ') + single_fields = cls._split_data_type(inner_type, ',') for single_field in single_fields: + single_field = single_field.strip() # If component type starts with a comma, the first single field # will be an empty string. Disregard this empty string. if not single_field: @@ -978,13 +979,13 @@ def _parse_structural_column(cls, column: RowProxy, result: List[dict]) -> None: stack.append((field_info[0], field_info[1])) full_parent_path = cls._get_full_name(stack) result.append(cls._create_column_info( - column, full_parent_path, + full_parent_path, presto_type_map[field_info[1]]())) else: # otherwise this field is a basic data type full_parent_path = cls._get_full_name(stack) column_name = '{}.{}'.format(full_parent_path, field_info[0]) result.append(cls._create_column_info( - column, column_name, presto_type_map[field_info[1]]())) + column_name, presto_type_map[field_info[1]]())) # If the component type ends with a structural data type, do not pop # the stack. We have run across a structural data type within the # overall structural data type. Otherwise, we have completely parsed @@ -1036,7 +1037,12 @@ def get_columns( try: # parse column if it is a row or array if 'array' in column.Type or 'row' in column.Type: - cls._parse_structural_column(column, result) + full_data_type = '{} {}'.format(column.Column, column.Type) + structural_column_index = len(result) + cls._parse_structural_column(full_data_type, result) + result[structural_column_index]['nullable'] = getattr( + column, 'Null', True) + result[structural_column_index]['default'] = None continue else: # otherwise column is a basic data type column_type = presto_type_map[column.Type]() @@ -1044,7 +1050,10 @@ def get_columns( logging.info('Did not recognize type {} of column {}'.format( column.Type, column.Column)) column_type = types.NullType - result.append(cls._create_column_info(column, column.Column, column_type)) + column_info = cls._create_column_info(column.Column, column_type) + column_info['nullable'] = getattr(column, 'Null', True) + column_info['default'] = None + result.append(column_info) return result @classmethod @@ -1089,18 +1098,12 @@ def _get_fields(cls, cols: List[dict]) -> List[ColumnClause]: return column_clauses @classmethod - def _filter_presto_cols(cls, cols: List[dict]) -> List[dict]: + def _filter_out_array_nested_cols( + cls, cols: List[dict]) -> Tuple[List[dict], List[dict]]: """ - We want to filter out columns that correspond to array content because expanding - arrays would require us to use unnest and join. This can lead to a large, - complicated, and slow query. - - Example: select array_content - from TABLE - cross join UNNEST(array_column) as t(array_content); - - We know which columns to skip because cols is a list provided to us in a specific - order where a structural column is positioned right before its content. + Filter out columns that correspond to array content. We know which columns to + skip because cols is a list provided to us in a specific order where a structural + column is positioned right before its content. Example: Column Name: ColA, Column Data Type: array(row(nest_obj int)) cols = [ ..., ColA, ColA.nest_obj, ... ] @@ -1108,23 +1111,26 @@ def _filter_presto_cols(cls, cols: List[dict]) -> List[dict]: When we run across an array, check if subsequent column names start with the array name and skip them. :param cols: columns - :return: filtered list of columns + :return: filtered list of columns and list of array columns and its nested fields """ filtered_cols = [] - curr_array_col_name = '' + array_cols = [] + curr_array_col_name = None for col in cols: # col corresponds to an array's content and should be skipped if curr_array_col_name and col['name'].startswith(curr_array_col_name): + array_cols.append(col) continue # col is an array so we need to check if subsequent # columns correspond to the array's contents elif str(col['type']) == 'ARRAY': curr_array_col_name = col['name'] + array_cols.append(col) filtered_cols.append(col) else: - curr_array_col_name = '' + curr_array_col_name = None filtered_cols.append(col) - return filtered_cols + return filtered_cols, array_cols @classmethod def select_star(cls, my_db, table_name: str, engine: Engine, schema: str = None, @@ -1137,7 +1143,9 @@ def select_star(cls, my_db, table_name: str, engine: Engine, schema: str = None, """ presto_cols = cols if show_cols: - presto_cols = cls._filter_presto_cols(cols) + dot_regex = r'\.(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)' + presto_cols = [ + col for col in presto_cols if not re.search(dot_regex, col['name'])] return super(PrestoEngineSpec, cls).select_star( my_db, table_name, engine, schema, limit, show_cols, indent, latest_partition, presto_cols, @@ -1185,6 +1193,370 @@ def get_all_datasource_names(cls, db, datasource_type: str) \ schema=row['table_schema'], table=row['table_name'])) return datasource_names + @classmethod + def _build_column_hierarchy(cls, + columns: List[dict], + parent_column_types: List[str], + column_hierarchy: dict) -> None: + """ + Build a graph where the root node represents a column whose data type is in + parent_column_types. A node's children represent that column's nested fields + :param columns: list of columns + :param parent_column_types: list of data types that decide what columns can + be root nodes + :param column_hierarchy: dictionary representing the graph + """ + if len(columns) == 0: + return + root = columns.pop(0) + root_info = {'type': root['type'], 'children': []} + column_hierarchy[root['name']] = root_info + while columns: + column = columns[0] + # If the column name does not start with the root's name, + # then this column is not a nested field + if not column['name'].startswith(f"{root['name']}."): + break + # If the column's data type is one of the parent types, + # then this column may have nested fields + if str(column['type']) in parent_column_types: + cls._build_column_hierarchy(columns, parent_column_types, + column_hierarchy) + root_info['children'].append(column['name']) + continue + else: # The column is a nested field + root_info['children'].append(column['name']) + columns.pop(0) + + @classmethod + def _create_row_and_array_hierarchy( + cls, selected_columns: List[dict]) -> Tuple[dict, dict, List[dict]]: + """ + Build graphs where the root node represents a row or array and its children + are that column's nested fields + :param selected_columns: columns selected in a query + :return: graph representing a row, graph representing an array, and a list + of all the nested fields + """ + row_column_hierarchy: OrderedDict = OrderedDict() + array_column_hierarchy: OrderedDict = OrderedDict() + expanded_columns: List[dict] = [] + for column in selected_columns: + if column['type'].startswith('ROW'): + parsed_row_columns: List[dict] = [] + full_data_type = '{} {}'.format(column['name'], column['type'].lower()) + cls._parse_structural_column(full_data_type, parsed_row_columns) + expanded_columns = expanded_columns + parsed_row_columns[1:] + filtered_row_columns, array_columns = cls._filter_out_array_nested_cols( + parsed_row_columns) + cls._build_column_hierarchy(filtered_row_columns, + ['ROW'], + row_column_hierarchy) + cls._build_column_hierarchy(array_columns, + ['ROW', 'ARRAY'], + array_column_hierarchy) + elif column['type'].startswith('ARRAY'): + parsed_array_columns: List[dict] = [] + full_data_type = '{} {}'.format(column['name'], column['type'].lower()) + cls._parse_structural_column(full_data_type, parsed_array_columns) + expanded_columns = expanded_columns + parsed_array_columns[1:] + cls._build_column_hierarchy(parsed_array_columns, + ['ROW', 'ARRAY'], + array_column_hierarchy) + return row_column_hierarchy, array_column_hierarchy, expanded_columns + + @classmethod + def _create_empty_row_of_data(cls, columns: List[dict]) -> dict: + """ + Create an empty row of data + :param columns: list of columns + :return: dictionary representing an empty row of data + """ + return {column['name']: '' for column in columns} + + @classmethod + def _expand_row_data(cls, datum: dict, column: str, column_hierarchy: dict) -> None: + """ + Separate out nested fields and its value in a row of data + :param datum: row of data + :param column: row column name + :param column_hierarchy: dictionary tracking structural columns and its + nested fields + """ + if column in datum: + row_data = datum[column] + row_children = column_hierarchy[column]['children'] + if row_data and len(row_data) != len(row_children): + raise Exception('The number of data values and number of nested' + 'fields are not equal') + elif row_data: + for index, data_value in enumerate(row_data): + datum[row_children[index]] = data_value + else: + for row_child in row_children: + datum[row_child] = '' + + @classmethod + def _split_array_columns_by_process_state( + cls, array_columns: List[str], + array_column_hierarchy: dict, + datum: dict) -> Tuple[List[str], Set[str]]: + """ + Take a list of array columns and split them according to whether or not we are + ready to process them from a data set + :param array_columns: list of array columns + :param array_column_hierarchy: graph representing array columns + :param datum: row of data + :return: list of array columns ready to be processed and set of array columns + not ready to be processed + """ + array_columns_to_process = [] + unprocessed_array_columns = set() + child_array = None + for array_column in array_columns: + if array_column in datum: + array_columns_to_process.append(array_column) + elif str(array_column_hierarchy[array_column]['type']) == 'ARRAY': + child_array = array_column + unprocessed_array_columns.add(child_array) + elif child_array and array_column.startswith(child_array): + unprocessed_array_columns.add(array_column) + return array_columns_to_process, unprocessed_array_columns + + @classmethod + def _convert_data_list_to_array_data_dict( + cls, data: List[dict], array_columns_to_process: List[str]) -> dict: + """ + Pull out array data from rows of data into a dictionary where the key represents + the index in the data list and the value is the array data values + Example: + data = [ + {'ColumnA': [1, 2], 'ColumnB': 3}, + {'ColumnA': [11, 22], 'ColumnB': 3} + ] + data dictionary = { + 0: [{'ColumnA': [1, 2]], + 1: [{'ColumnA': [11, 22]] + } + :param data: rows of data + :param array_columns_to_process: array columns we want to pull out + :return: data dictionary + """ + array_data_dict = {} + for data_index, datum in enumerate(data): + all_array_datum = {} + for array_column in array_columns_to_process: + all_array_datum[array_column] = datum[array_column] + array_data_dict[data_index] = [all_array_datum] + return array_data_dict + + @classmethod + def _process_array_data(cls, + data: List[dict], + all_columns: List[dict], + array_column_hierarchy: dict) -> dict: + """ + Pull out array data that is ready to be processed into a dictionary. + The key refers to the index in the original data set. The value is + a list of data values. Initially this list will contain just one value, + the row of data that corresponds to the index in the original data set. + As we process arrays, we will pull out array values into separate rows + and append them to the list of data values. + Example: + Original data set = [ + {'ColumnA': [1, 2], 'ColumnB': [3]}, + {'ColumnA': [11, 22], 'ColumnB': [33]} + ] + all_array_data (intially) = { + 0: [{'ColumnA': [1, 2], 'ColumnB': [3}], + 1: [{'ColumnA': [11, 22], 'ColumnB': [33]}] + } + all_array_data (after processing) = { + 0: [ + {'ColumnA': 1, 'ColumnB': 3}, + {'ColumnA': 2, 'ColumnB': ''}, + ], + 1: [ + {'ColumnA': 11, 'ColumnB': 33}, + {'ColumnA': 22, 'ColumnB': ''}, + ], + } + :param data: rows of data + :param all_columns: list of columns + :param array_column_hierarchy: graph representing array columns + :return: dictionary representing processed array data + """ + array_columns = list(array_column_hierarchy.keys()) + # Determine what columns are ready to be processed. This is necessary for + # array columns that contain rows with nested arrays. We first process + # the outer arrays before processing inner arrays. + array_columns_to_process, \ + unprocessed_array_columns = cls._split_array_columns_by_process_state( + array_columns, array_column_hierarchy, data[0]) + + # Pull out array data that is ready to be processed into a dictionary. + all_array_data = cls._convert_data_list_to_array_data_dict( + data, array_columns_to_process) + + for original_data_index, expanded_array_data in all_array_data.items(): + for array_column in array_columns: + if array_column in unprocessed_array_columns: + continue + # Expand array values that are rows + if str(array_column_hierarchy[array_column]['type']) == 'ROW': + for array_value in expanded_array_data: + cls._expand_row_data(array_value, + array_column, + array_column_hierarchy) + continue + array_data = expanded_array_data[0][array_column] + array_children = array_column_hierarchy[array_column] + # This is an empty array of primitive data type + if not array_data and not array_children['children']: + continue + # Pull out complex array values into its own row of data + elif array_data and array_children['children']: + for array_index, data_value in enumerate(array_data): + if array_index >= len(expanded_array_data): + empty_data = cls._create_empty_row_of_data(all_columns) + expanded_array_data.append(empty_data) + for index, datum_value in enumerate(data_value): + array_child = array_children['children'][index] + expanded_array_data[array_index][array_child] = datum_value + # Pull out primitive array values into its own row of data + elif array_data: + for array_index, data_value in enumerate(array_data): + if array_index >= len(expanded_array_data): + empty_data = cls._create_empty_row_of_data(all_columns) + expanded_array_data.append(empty_data) + expanded_array_data[array_index][array_column] = data_value + # This is an empty array with nested fields + else: + for index, array_child in enumerate(array_children['children']): + for array_value in expanded_array_data: + array_value[array_child] = '' + return all_array_data + + @classmethod + def _consolidate_array_data_into_data(cls, + data: List[dict], + array_data: dict) -> None: + """ + Consolidate data given a list representing rows of data and a dictionary + representing expanded array data + Example: + Original data set = [ + {'ColumnA': [1, 2], 'ColumnB': [3]}, + {'ColumnA': [11, 22], 'ColumnB': [33]} + ] + array_data = { + 0: [ + {'ColumnA': 1, 'ColumnB': 3}, + {'ColumnA': 2, 'ColumnB': ''}, + ], + 1: [ + {'ColumnA': 11, 'ColumnB': 33}, + {'ColumnA': 22, 'ColumnB': ''}, + ], + } + Final data set = [ + {'ColumnA': 1, 'ColumnB': 3}, + {'ColumnA': 2, 'ColumnB': ''}, + {'ColumnA': 11, 'ColumnB': 33}, + {'ColumnA': 22, 'ColumnB': ''}, + ] + :param data: list representing rows of data + :param array_data: dictionary representing expanded array data + :return: list where data and array_data are combined + """ + data_index = 0 + original_data_index = 0 + while data_index < len(data): + data[data_index].update(array_data[original_data_index][0]) + array_data[original_data_index].pop(0) + data[data_index + 1:data_index + 1] = array_data[original_data_index] + data_index = data_index + len(array_data[original_data_index]) + 1 + original_data_index = original_data_index + 1 + + @classmethod + def _remove_processed_array_columns(cls, + unprocessed_array_columns: Set[str], + array_column_hierarchy: dict) -> None: + """ + Remove keys representing array columns that have already been processed + :param unprocessed_array_columns: list of unprocessed array columns + :param array_column_hierarchy: graph representing array columns + """ + array_columns = list(array_column_hierarchy.keys()) + for array_column in array_columns: + if array_column in unprocessed_array_columns: + continue + else: + del array_column_hierarchy[array_column] + + @classmethod + def expand_data(cls, + columns: List[dict], + data: List[dict]) -> Tuple[List[dict], List[dict], List[dict]]: + """ + We do not immediately display rows and arrays clearly in the data grid. This + method separates out nested fields and data values to help clearly display + structural columns. + + Example: ColumnA is a row(nested_obj varchar) and ColumnB is an array(int) + Original data set = [ + {'ColumnA': ['a1'], 'ColumnB': [1, 2]}, + {'ColumnA': ['a2'], 'ColumnB': [3, 4]}, + ] + Expanded data set = [ + {'ColumnA': ['a1'], 'ColumnA.nested_obj': 'a1', 'ColumnB': 1}, + {'ColumnA': '', 'ColumnA.nested_obj': '', 'ColumnB': 2}, + {'ColumnA': ['a2'], 'ColumnA.nested_obj': 'a2', 'ColumnB': 3}, + {'ColumnA': '', 'ColumnA.nested_obj': '', 'ColumnB': 4}, + ] + :param columns: columns selected in the query + :param data: original data set + :return: list of all columns(selected columns and their nested fields), + expanded data set, listed of nested fields + """ + all_columns: List[dict] = [] + # Get the list of all columns (selected fields and their nested fields) + for column in columns: + if column['type'].startswith('ARRAY') or column['type'].startswith('ROW'): + full_data_type = '{} {}'.format(column['name'], column['type'].lower()) + cls._parse_structural_column(full_data_type, all_columns) + else: + all_columns.append(column) + + # Build graphs where the root node is a row or array and its children are that + # column's nested fields + row_column_hierarchy,\ + array_column_hierarchy,\ + expanded_columns = cls._create_row_and_array_hierarchy(columns) + + # Pull out a row's nested fields and their values into separate columns + ordered_row_columns = row_column_hierarchy.keys() + for datum in data: + for row_column in ordered_row_columns: + cls._expand_row_data(datum, row_column, row_column_hierarchy) + + while array_column_hierarchy: + array_columns = list(array_column_hierarchy.keys()) + # Determine what columns are ready to be processed. + array_columns_to_process,\ + unprocessed_array_columns = cls._split_array_columns_by_process_state( + array_columns, array_column_hierarchy, data[0]) + all_array_data = cls._process_array_data(data, + all_columns, + array_column_hierarchy) + # Consolidate the original data set and the expanded array data + cls._consolidate_array_data_into_data(data, all_array_data) + # Remove processed array columns from the graph + cls._remove_processed_array_columns(unprocessed_array_columns, + array_column_hierarchy) + + return all_columns, data, expanded_columns + @classmethod def extra_table_metadata(cls, database, table_name, schema_name): indexes = database.get_indexes(table_name, schema_name) diff --git a/superset/sql_lab.py b/superset/sql_lab.py index 6a5ee8ebc53ae..86e171ba44d77 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -279,10 +279,17 @@ def execute_sql_statements( latest_partition=False) query.end_time = now_as_float() + selected_columns = cdf.columns or [] + data = cdf.data or [] + all_columns, data, expanded_columns = db_engine_spec.expand_data( + selected_columns, data) + payload.update({ 'status': query.status, - 'data': cdf.data if cdf.data else [], - 'columns': cdf.columns if cdf.columns else [], + 'data': data, + 'columns': all_columns, + 'selected_columns': selected_columns, + 'expanded_columns': expanded_columns, 'query': query.to_dict(), }) diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index 43f89c1435394..02dbbae8bcfed 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -399,13 +399,286 @@ def test_presto_get_fields(self): self.assertEqual(actual_result.element.name, expected_result['name']) self.assertEqual(actual_result.name, expected_result['label']) - def test_presto_filter_presto_cols(self): + def test_presto_filter_out_array_nested_cols(self): cols = [ {'name': 'column', 'type': 'ARRAY'}, {'name': 'column.nested_obj', 'type': 'FLOAT'}] - actual_results = PrestoEngineSpec._filter_presto_cols(cols) - expected_results = [cols[0]] - self.assertEqual(actual_results, expected_results) + actual_filtered_cols,\ + actual_array_cols = PrestoEngineSpec._filter_out_array_nested_cols(cols) + expected_filtered_cols = [{'name': 'column', 'type': 'ARRAY'}] + self.assertEqual(actual_filtered_cols, expected_filtered_cols) + self.assertEqual(actual_array_cols, cols) + + def test_presto_create_row_and_array_hierarchy(self): + cols = [ + {'name': 'row_column', + 'type': 'ROW(NESTED_OBJ1 VARCHAR, NESTED_ROW ROW(NESTED_OBJ2 VARCHAR)'}, + {'name': 'array_column', + 'type': 'ARRAY(ROW(NESTED_ARRAY ARRAY(ROW(NESTED_OBJ VARCHAR))))'}] + actual_row_col_hierarchy,\ + actual_array_col_hierarchy,\ + actual_expanded_cols = PrestoEngineSpec._create_row_and_array_hierarchy(cols) + expected_row_col_hierarchy = { + 'row_column': { + 'type': 'ROW', + 'children': ['row_column.nested_obj1', 'row_column.nested_row'], + }, + 'row_column.nested_row': { + 'type': 'ROW', + 'children': ['row_column.nested_row.nested_obj2']}, + } + expected_array_col_hierarchy = { + 'array_column': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array'], + }, + 'array_column.nested_array': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array.nested_obj']}, + } + expected_expanded_cols = [ + {'name': 'row_column.nested_obj1', 'type': 'VARCHAR'}, + {'name': 'row_column.nested_row', 'type': 'ROW'}, + {'name': 'row_column.nested_row.nested_obj2', 'type': 'VARCHAR'}, + {'name': 'array_column.nested_array', 'type': 'ARRAY'}, + {'name': 'array_column.nested_array.nested_obj', 'type': 'VARCHAR'}] + self.assertEqual(actual_row_col_hierarchy, expected_row_col_hierarchy) + self.assertEqual(actual_array_col_hierarchy, expected_array_col_hierarchy) + self.assertEqual(actual_expanded_cols, expected_expanded_cols) + + def test_presto_expand_row_data(self): + datum = {'row_col': [1, 'a']} + row_column = 'row_col' + row_col_hierarchy = { + 'row_col': { + 'type': 'ROW', + 'children': ['row_col.nested_int', 'row_col.nested_str'], + }, + } + PrestoEngineSpec._expand_row_data(datum, row_column, row_col_hierarchy) + expected_datum = { + 'row_col': [1, 'a'], 'row_col.nested_int': 1, 'row_col.nested_str': 'a', + } + self.assertEqual(datum, expected_datum) + + def test_split_array_columns_by_process_state(self): + array_cols = ['array_column', 'array_column.nested_array'] + array_col_hierarchy = { + 'array_column': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array'], + }, + 'array_column.nested_array': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array.nested_obj']}, + } + datum = {'array_column': [[[1], [2]]]} + actual_array_cols_to_process, actual_unprocessed_array_cols = \ + PrestoEngineSpec._split_array_columns_by_process_state( + array_cols, array_col_hierarchy, datum) + expected_array_cols_to_process = ['array_column'] + expected_unprocessed_array_cols = {'array_column.nested_array'} + self.assertEqual(actual_array_cols_to_process, expected_array_cols_to_process) + self.assertEqual(actual_unprocessed_array_cols, expected_unprocessed_array_cols) + + def test_presto_convert_data_list_to_array_data_dict(self): + data = [ + {'array_column': [1, 2], 'int_column': 3}, + {'array_column': [11, 22], 'int_column': 33}, + ] + array_columns_to_process = ['array_column'] + actual_array_data_dict = PrestoEngineSpec._convert_data_list_to_array_data_dict( + data, array_columns_to_process) + expected_array_data_dict = { + 0: [{'array_column': [1, 2]}], + 1: [{'array_column': [11, 22]}]} + self.assertEqual(actual_array_data_dict, expected_array_data_dict) + + def test_presto_process_array_data(self): + data = [ + {'array_column': [[1], [2]], 'int_column': 3}, + {'array_column': [[11], [22]], 'int_column': 33}, + ] + all_columns = [ + {'name': 'array_column', 'type': 'ARRAY'}, + {'name': 'array_column.nested_row', 'type': 'BIGINT'}, + {'name': 'int_column', 'type': 'BIGINT'}, + ] + array_column_hierarchy = { + 'array_column': { + 'type': 'ARRAY', + 'children': ['array_column.nested_row'], + }, + } + actual_array_data = PrestoEngineSpec._process_array_data( + data, all_columns, array_column_hierarchy) + expected_array_data = { + 0: [ + {'array_column': [[1], [2]], 'array_column.nested_row': 1}, + {'array_column': '', 'array_column.nested_row': 2, 'int_column': ''}, + ], + 1: [ + {'array_column': [[11], [22]], 'array_column.nested_row': 11}, + {'array_column': '', 'array_column.nested_row': 22, 'int_column': ''}, + ], + } + self.assertEqual(actual_array_data, expected_array_data) + + def test_presto_consolidate_array_data_into_data(self): + data = [ + {'arr_col': [[1], [2]], 'int_col': 3}, + {'arr_col': [[11], [22]], 'int_col': 33}, + ] + array_data = { + 0: [ + {'arr_col': [[1], [2]], 'arr_col.nested_row': 1}, + {'arr_col': '', 'arr_col.nested_row': 2, 'int_col': ''}, + ], + 1: [ + {'arr_col': [[11], [22]], 'arr_col.nested_row': 11}, + {'arr_col': '', 'arr_col.nested_row': 22, 'int_col': ''}, + ], + } + PrestoEngineSpec._consolidate_array_data_into_data(data, array_data) + expected_data = [ + {'arr_col': [[1], [2]], 'arr_col.nested_row': 1, 'int_col': 3}, + {'arr_col': '', 'arr_col.nested_row': 2, 'int_col': ''}, + {'arr_col': [[11], [22]], 'arr_col.nested_row': 11, 'int_col': 33}, + {'arr_col': '', 'arr_col.nested_row': 22, 'int_col': ''}, + ] + self.assertEqual(data, expected_data) + + def test_presto_remove_processed_array_columns(self): + array_col_hierarchy = { + 'array_column': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array'], + }, + 'array_column.nested_array': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array.nested_obj']}, + } + unprocessed_array_cols = {'array_column.nested_array'} + PrestoEngineSpec._remove_processed_array_columns( + unprocessed_array_cols, array_col_hierarchy) + expected_array_col_hierarchy = { + 'array_column.nested_array': { + 'type': 'ARRAY', + 'children': ['array_column.nested_array.nested_obj']}, + } + self.assertEqual(array_col_hierarchy, expected_array_col_hierarchy) + + def test_presto_expand_data_with_simple_structural_columns(self): + cols = [ + {'name': 'row_column', 'type': 'ROW(NESTED_OBJ VARCHAR)'}, + {'name': 'array_column', 'type': 'ARRAY(BIGINT)'}] + data = [ + {'row_column': ['a'], 'array_column': [1, 2, 3]}, + {'row_column': ['b'], 'array_column': [4, 5, 6]}] + actual_cols, actual_data, actual_expanded_cols = PrestoEngineSpec.expand_data( + cols, data) + expected_cols = [ + {'name': 'row_column', 'type': 'ROW'}, + {'name': 'row_column.nested_obj', 'type': 'VARCHAR'}, + {'name': 'array_column', 'type': 'ARRAY'}] + expected_data = [ + {'row_column': ['a'], 'row_column.nested_obj': 'a', 'array_column': 1}, + {'row_column': '', 'row_column.nested_obj': '', 'array_column': 2}, + {'row_column': '', 'row_column.nested_obj': '', 'array_column': 3}, + {'row_column': ['b'], 'row_column.nested_obj': 'b', 'array_column': 4}, + {'row_column': '', 'row_column.nested_obj': '', 'array_column': 5}, + {'row_column': '', 'row_column.nested_obj': '', 'array_column': 6}] + expected_expanded_cols = [ + {'name': 'row_column.nested_obj', 'type': 'VARCHAR'}] + self.assertEqual(actual_cols, expected_cols) + self.assertEqual(actual_data, expected_data) + self.assertEqual(actual_expanded_cols, expected_expanded_cols) + + def test_presto_expand_data_with_complex_row_columns(self): + cols = [ + {'name': 'row_column', + 'type': 'ROW(NESTED_OBJ1 VARCHAR, NESTED_ROW ROW(NESTED_OBJ2 VARCHAR)'}] + data = [ + {'row_column': ['a1', ['a2']]}, + {'row_column': ['b1', ['b2']]}] + actual_cols, actual_data, actual_expanded_cols = PrestoEngineSpec.expand_data( + cols, data) + expected_cols = [ + {'name': 'row_column', 'type': 'ROW'}, + {'name': 'row_column.nested_obj1', 'type': 'VARCHAR'}, + {'name': 'row_column.nested_row', 'type': 'ROW'}, + {'name': 'row_column.nested_row.nested_obj2', 'type': 'VARCHAR'}] + expected_data = [ + {'row_column': ['a1', ['a2']], + 'row_column.nested_obj1': 'a1', + 'row_column.nested_row': ['a2'], + 'row_column.nested_row.nested_obj2': 'a2'}, + {'row_column': ['b1', ['b2']], + 'row_column.nested_obj1': 'b1', + 'row_column.nested_row': ['b2'], + 'row_column.nested_row.nested_obj2': 'b2'}] + expected_expanded_cols = [ + {'name': 'row_column.nested_obj1', 'type': 'VARCHAR'}, + {'name': 'row_column.nested_row', 'type': 'ROW'}, + {'name': 'row_column.nested_row.nested_obj2', 'type': 'VARCHAR'}] + self.assertEqual(actual_cols, expected_cols) + self.assertEqual(actual_data, expected_data) + self.assertEqual(actual_expanded_cols, expected_expanded_cols) + + def test_presto_expand_data_with_complex_array_columns(self): + cols = [ + {'name': 'int_column', 'type': 'BIGINT'}, + {'name': 'array_column', + 'type': 'ARRAY(ROW(NESTED_ARRAY ARRAY(ROW(NESTED_OBJ VARCHAR))))'}] + data = [ + {'int_column': 1, 'array_column': [[[['a'], ['b']]], [[['c'], ['d']]]]}, + {'int_column': 2, 'array_column': [[[['e'], ['f']]], [[['g'], ['h']]]]}] + actual_cols, actual_data, actual_expanded_cols = PrestoEngineSpec.expand_data( + cols, data) + expected_cols = [ + {'name': 'int_column', 'type': 'BIGINT'}, + {'name': 'array_column', 'type': 'ARRAY'}, + {'name': 'array_column.nested_array', 'type': 'ARRAY'}, + {'name': 'array_column.nested_array.nested_obj', 'type': 'VARCHAR'}] + expected_data = [ + {'int_column': 1, + 'array_column': [[[['a'], ['b']]], [[['c'], ['d']]]], + 'array_column.nested_array': [['a'], ['b']], + 'array_column.nested_array.nested_obj': 'a'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': '', + 'array_column.nested_array.nested_obj': 'b'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': [['c'], ['d']], + 'array_column.nested_array.nested_obj': 'c'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': '', + 'array_column.nested_array.nested_obj': 'd'}, + {'int_column': 2, + 'array_column': [[[['e'], ['f']]], [[['g'], ['h']]]], + 'array_column.nested_array': [['e'], ['f']], + 'array_column.nested_array.nested_obj': 'e'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': '', + 'array_column.nested_array.nested_obj': 'f'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': [['g'], ['h']], + 'array_column.nested_array.nested_obj': 'g'}, + {'int_column': '', + 'array_column': '', + 'array_column.nested_array': '', + 'array_column.nested_array.nested_obj': 'h'}] + expected_expanded_cols = [ + {'name': 'array_column.nested_array', 'type': 'ARRAY'}, + {'name': 'array_column.nested_array.nested_obj', 'type': 'VARCHAR'}] + self.assertEqual(actual_cols, expected_cols) + self.assertEqual(actual_data, expected_data) + self.assertEqual(actual_expanded_cols, expected_expanded_cols) def test_hive_get_view_names_return_empty_list(self): self.assertEquals([], HiveEngineSpec.get_view_names(mock.ANY, mock.ANY)) From 9acafd5b759db42b2f3477b35f51de85a89b61fe Mon Sep 17 00:00:00 2001 From: Kim Truong <47833996+khtruong@users.noreply.github.com> Date: Fri, 31 May 2019 13:38:05 -0700 Subject: [PATCH 34/35] fix: handle presto columns with whitespace (#7630) --- superset/db_engine_specs.py | 34 +++++++++++++++++++++++++--------- tests/db_engine_specs_test.py | 9 ++++++++- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/superset/db_engine_specs.py b/superset/db_engine_specs.py index 00a9310b2acf4..89e677b0136ea 100644 --- a/superset/db_engine_specs.py +++ b/superset/db_engine_specs.py @@ -945,11 +945,20 @@ def _split_data_type(cls, data_type: str, delimiter: str) -> List[str]: r'{}(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)'.format(delimiter), data_type) @classmethod - def _parse_structural_column(cls, full_data_type: str, result: List[dict]) -> None: + def _parse_structural_column(cls, + parent_column_name: str, + parent_data_type: str, + result: List[dict]) -> None: """ Parse a row or array column :param result: list tracking the results """ + formatted_parent_column_name = parent_column_name + # Quote the column name if there is a space + if ' ' in parent_column_name: + formatted_parent_column_name = f'"{parent_column_name}"' + full_data_type = f'{formatted_parent_column_name} {parent_data_type}' + original_result_len = len(result) # split on open parenthesis ( to get the structural # data type and its component types data_types = cls._split_data_type(full_data_type, r'\(') @@ -1001,6 +1010,11 @@ def _parse_structural_column(cls, full_data_type: str, result: List[dict]) -> No # Because it is an array of a basic data type. We have finished # parsing the structural data type and can move on. stack.pop() + # Unquote the column name if necessary + if formatted_parent_column_name != parent_column_name: + for index in range(original_result_len, len(result)): + result[index]['name'] = result[index]['name'].replace( + formatted_parent_column_name, parent_column_name) @classmethod def _show_columns( @@ -1037,9 +1051,8 @@ def get_columns( try: # parse column if it is a row or array if 'array' in column.Type or 'row' in column.Type: - full_data_type = '{} {}'.format(column.Column, column.Type) structural_column_index = len(result) - cls._parse_structural_column(full_data_type, result) + cls._parse_structural_column(column.Column, column.Type, result) result[structural_column_index]['nullable'] = getattr( column, 'Null', True) result[structural_column_index]['default'] = None @@ -1244,8 +1257,9 @@ def _create_row_and_array_hierarchy( for column in selected_columns: if column['type'].startswith('ROW'): parsed_row_columns: List[dict] = [] - full_data_type = '{} {}'.format(column['name'], column['type'].lower()) - cls._parse_structural_column(full_data_type, parsed_row_columns) + cls._parse_structural_column(column['name'], + column['type'].lower(), + parsed_row_columns) expanded_columns = expanded_columns + parsed_row_columns[1:] filtered_row_columns, array_columns = cls._filter_out_array_nested_cols( parsed_row_columns) @@ -1257,8 +1271,9 @@ def _create_row_and_array_hierarchy( array_column_hierarchy) elif column['type'].startswith('ARRAY'): parsed_array_columns: List[dict] = [] - full_data_type = '{} {}'.format(column['name'], column['type'].lower()) - cls._parse_structural_column(full_data_type, parsed_array_columns) + cls._parse_structural_column(column['name'], + column['type'].lower(), + parsed_array_columns) expanded_columns = expanded_columns + parsed_array_columns[1:] cls._build_column_hierarchy(parsed_array_columns, ['ROW', 'ARRAY'], @@ -1523,8 +1538,9 @@ def expand_data(cls, # Get the list of all columns (selected fields and their nested fields) for column in columns: if column['type'].startswith('ARRAY') or column['type'].startswith('ROW'): - full_data_type = '{} {}'.format(column['name'], column['type'].lower()) - cls._parse_structural_column(full_data_type, all_columns) + cls._parse_structural_column(column['name'], + column['type'].lower(), + all_columns) else: all_columns.append(column) diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index 02dbbae8bcfed..44919143d8c02 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -350,7 +350,14 @@ def test_presto_get_simple_row_column(self): ('column_name.nested_obj', 'FLOAT')] self.verify_presto_column(presto_column, expected_results) - def test_presto_get_simple_row_column_with_tricky_name(self): + def test_presto_get_simple_row_column_with_name_containing_whitespace(self): + presto_column = ('column name', 'row(nested_obj double)', '') + expected_results = [ + ('column name', 'ROW'), + ('column name.nested_obj', 'FLOAT')] + self.verify_presto_column(presto_column, expected_results) + + def test_presto_get_simple_row_column_with_tricky_nested_field_name(self): presto_column = ('column_name', 'row("Field Name(Tricky, Name)" double)', '') expected_results = [ ('column_name', 'ROW'), From c82a7f42e1d8415cdf43dd76f4bfdfd7436c84c8 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Fri, 31 May 2019 13:47:30 -0700 Subject: [PATCH 35/35] Update bug_report.md (#7583) --- .github/ISSUE_TEMPLATE/bug_report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 89395fb3d197e..388b5783a1752 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -29,7 +29,7 @@ If applicable, add screenshots to help explain your problem. (please complete the following information): -- superset version: [e.g. `v0.29`, `master`, `commit`] +- superset version: `superset version` - python version: `python --version` - node.js version: `node -v` - npm version: `npm -v`