diff --git a/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx b/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx
new file mode 100644
index 0000000000000..cdd0f22c006a1
--- /dev/null
+++ b/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx
@@ -0,0 +1,62 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+
+import { Label } from 'react-bootstrap';
+import LimitControl from '../../../src/SqlLab/components/LimitControl';
+import ControlHeader from '../../../src/explore/components/ControlHeader';
+ describe('LimitControl', () => {
+ const defaultProps = {
+ value: 0,
+ defaultQueryLimit: 1000,
+ maxRow: 100000,
+ onChange: () => {},
+ };
+ let wrapper;
+ const factory = o => ;
+ beforeEach(() => {
+ wrapper = shallow(factory(defaultProps));
+ });
+ it('is a valid element', () => {
+ expect(React.isValidElement()).toEqual(true);
+ });
+ it('renders a Label', () => {
+ expect(wrapper.find(Label)).toHaveLength(1);
+ });
+ it('loads the correct state', () => {
+ const value = 100;
+ wrapper = shallow(factory({ ...defaultProps, value }));
+ expect(wrapper.state().textValue).toEqual(value.toString());
+ wrapper.find(Label).first().simulate('click');
+ expect(wrapper.state().showOverlay).toBe(true);
+ expect(wrapper.find(ControlHeader).props().validationErrors).toHaveLength(0);
+ });
+ it('handles invalid value', () => {
+ wrapper.find(Label).first().simulate('click');
+ wrapper.setState({ textValue: 'invalid' });
+ expect(wrapper.find(ControlHeader).props().validationErrors).toHaveLength(1);
+ });
+ it('handles negative value', () => {
+ wrapper.find(Label).first().simulate('click');
+ wrapper.setState({ textValue: '-1' });
+ expect(wrapper.find(ControlHeader).props().validationErrors).toHaveLength(1);
+ });
+ it('handles value above max row', () => {
+ wrapper.find(Label).first().simulate('click');
+ wrapper.setState({ textValue: (defaultProps.maxRow + 1).toString() });
+ expect(wrapper.find(ControlHeader).props().validationErrors).toHaveLength(1);
+ });
+ it('opens and closes', () => {
+ wrapper.find(Label).first().simulate('click');
+ expect(wrapper.state().showOverlay).toBe(true);
+ wrapper.find('.ok').first().simulate('click');
+ expect(wrapper.state().showOverlay).toBe(false);
+ });
+ it('resets and closes', () => {
+ const value = 100;
+ wrapper = shallow(factory({ ...defaultProps, value }));
+ wrapper.find(Label).first().simulate('click');
+ expect(wrapper.state().textValue).toEqual(value.toString());
+ wrapper.find('.reset').simulate('click');
+ expect(wrapper.state().textValue).toEqual(defaultProps.defaultQueryLimit.toString());
+ });
+ });
diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx
index d1b58d32ed84e..81cbf20c96b6b 100644
--- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx
@@ -3,7 +3,8 @@ import { shallow } from 'enzyme';
import { describe, it } from 'mocha';
import { expect } from 'chai';
-import { initialState, queries, table } from './fixtures';
+import { defaultQueryEditor, initialState, queries, table } from './fixtures';
+import LimitControl from '../../../src/SqlLab/components/LimitControl';
import SqlEditor from '../../../src/SqlLab/components/SqlEditor';
import SqlEditorLeftBar from '../../../src/SqlLab/components/SqlEditorLeftBar';
@@ -18,6 +19,8 @@ describe('SqlEditor', () => {
getHeight: () => ('100px'),
editorQueries: [],
dataPreviewQueries: [],
+ defaultQueryLimit: 1000,
+ maxRow: 100000,
};
it('is valid', () => {
expect(
@@ -28,4 +31,18 @@ describe('SqlEditor', () => {
const wrapper = shallow();
expect(wrapper.find(SqlEditorLeftBar)).to.have.length(1);
});
+ it('render a LimitControl with default limit', () => {
+ const defaultQueryLimit = 101;
+ const updatedProps = { ...mockedProps, defaultQueryLimit };
+ const wrapper = shallow();
+ expect(wrapper.find(LimitControl)).toHaveLength(1);
+ expect(wrapper.find(LimitControl).props().value).toEqual(defaultQueryLimit);
+ });
+ it('render a LimitControl with existing limit', () => {
+ const queryEditor = { ...defaultQueryEditor, queryLimit: 101 };
+ const updatedProps = { ...mockedProps, queryEditor };
+ const wrapper = shallow();
+ expect(wrapper.find(LimitControl)).toHaveLength(1);
+ expect(wrapper.find(LimitControl).props().value).toEqual(queryEditor.queryLimit);
+ });
});
diff --git a/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx b/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx
index c898662f573dd..6671391b86623 100644
--- a/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx
@@ -52,6 +52,8 @@ describe('TabbedSqlEditors', () => {
editorHeight: '',
getHeight: () => ('100px'),
database: {},
+ defaultQueryLimit: 1000,
+ maxRow: 100000,
};
const getWrapper = () => (
shallow(, {
diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js
index c05a745884db7..fc68f76f6a52c 100644
--- a/superset/assets/spec/javascripts/sqllab/fixtures.js
+++ b/superset/assets/spec/javascripts/sqllab/fixtures.js
@@ -328,6 +328,12 @@ export const initialState = {
workspaceQueries: [],
queriesLastUpdate: 0,
activeSouthPaneTab: 'Results',
+ common: {
+ conf: {
+ DEFAULT_SQLLAB_LIMIT: 1000,
+ SQL_MAX_ROW: 100000,
+ },
+ },
};
export const query = {
diff --git a/superset/assets/spec/javascripts/sqllab/reducers_spec.js b/superset/assets/spec/javascripts/sqllab/reducers_spec.js
index a23ceb5d57b8f..747ba57eeddb2 100644
--- a/superset/assets/spec/javascripts/sqllab/reducers_spec.js
+++ b/superset/assets/spec/javascripts/sqllab/reducers_spec.js
@@ -91,6 +91,11 @@ describe('sqlLabReducer', () => {
newState = r.sqlLabReducer(newState, actions.queryEditorSetSql(qe, sql));
expect(newState.queryEditors[1].sql).to.equal(sql);
});
+ it('should not fail while setting queryLimit', () => {
+ const queryLimit = 101;
+ newState = sqlLabReducer(newState, actions.queryEditorSetQueryLimit(qe, queryLimit));
+ expect(newState.queryEditors[1].queryLimit).toEqual(queryLimit);
+ });
it('should set selectedText', () => {
const selectedText = 'TEST';
expect(newState.queryEditors[0].selectedText).to.equal(null);
diff --git a/superset/assets/src/SqlLab/actions.js b/superset/assets/src/SqlLab/actions.js
index 644947023bcb8..68bdbd9b00a8b 100644
--- a/superset/assets/src/SqlLab/actions.js
+++ b/superset/assets/src/SqlLab/actions.js
@@ -21,6 +21,7 @@ export const QUERY_EDITOR_SET_SCHEMA = 'QUERY_EDITOR_SET_SCHEMA';
export const QUERY_EDITOR_SET_TITLE = 'QUERY_EDITOR_SET_TITLE';
export const QUERY_EDITOR_SET_AUTORUN = 'QUERY_EDITOR_SET_AUTORUN';
export const QUERY_EDITOR_SET_SQL = 'QUERY_EDITOR_SET_SQL';
+export const QUERY_EDITOR_SET_QUERY_LIMIT = 'QUERY_EDITOR_SET_QUERY_LIMIT';
export const QUERY_EDITOR_SET_TEMPLATE_PARAMS = 'QUERY_EDITOR_SET_TEMPLATE_PARAMS';
export const QUERY_EDITOR_SET_SELECTED_TEXT = 'QUERY_EDITOR_SET_SELECTED_TEXT';
export const QUERY_EDITOR_PERSIST_HEIGHT = 'QUERY_EDITOR_PERSIST_HEIGHT';
@@ -135,6 +136,7 @@ export function runQuery(query) {
tmp_table_name: query.tempTableName,
select_as_cta: query.ctas,
templateParams: query.templateParams,
+ ueryLimit: query.queryLimit,
};
const sqlJsonUrl = '/superset/sql_json/' + location.search;
$.ajax({
@@ -254,6 +256,10 @@ export function queryEditorSetSql(queryEditor, sql) {
return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql };
}
+export function queryEditorSetQueryLimit(queryEditor, queryLimit) {
+ return { type: QUERY_EDITOR_SET_QUERY_LIMIT, queryEditor, queryLimit };
+}
+
export function queryEditorSetTemplateParams(queryEditor, templateParams) {
return { type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, queryEditor, templateParams };
}
@@ -339,6 +345,7 @@ export function reFetchQueryResults(query) {
tab: '',
runAsync: false,
ctas: false,
+ queryLimit: query.queryLimit,
};
dispatch(runQuery(newQuery));
dispatch(changeDataPreviewId(query.id, newQuery));
diff --git a/superset/assets/src/SqlLab/components/LimitControl.jsx b/superset/assets/src/SqlLab/components/LimitControl.jsx
new file mode 100644
index 0000000000000..11f627a60f37f
--- /dev/null
+++ b/superset/assets/src/SqlLab/components/LimitControl.jsx
@@ -0,0 +1,115 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import {
+ Button,
+ Label,
+ FormGroup,
+ FormControl,
+ Overlay,
+ Popover,
+} from 'react-bootstrap';
+import { t } from '@superset-ui/translation';
+ import ControlHeader from '../../explore/components/ControlHeader';
+ const propTypes = {
+ value: PropTypes.number,
+ defaultQueryLimit: PropTypes.number.isRequired,
+ maxRow: PropTypes.number.isRequired,
+ onChange: PropTypes.func.isRequired,
+ };
+ export default class LimitControl extends React.PureComponent {
+ constructor(props) {
+ super(props);
+ const { value, defaultQueryLimit } = props;
+ this.state = {
+ textValue: value.toString() || defaultQueryLimit.toString(),
+ showOverlay: false,
+ };
+ this.handleHide = this.handleHide.bind(this);
+ this.handleToggle = this.handleToggle.bind(this);
+ this.submitAndClose = this.submitAndClose.bind(this);
+ }
+ setValueAndClose(val) {
+ this.setState({ textValue: val }, this.submitAndClose);
+ }
+ submitAndClose() {
+ const value = parseInt(this.state.textValue, 10) || this.props.defaultQueryLimit;
+ this.props.onChange(value);
+ this.setState({ showOverlay: false });
+ }
+ isValidLimit(limit) {
+ const value = parseInt(limit, 10);
+ return !(Number.isNaN(value) || value <= 0 || (this.props.maxRow && value > this.props.maxRow));
+ }
+ handleToggle() {
+ this.setState({ showOverlay: !this.state.showOverlay });
+ }
+ handleHide() {
+ this.setState({ showOverlay: false });
+ }
+ renderPopover() {
+ const textValue = this.state.textValue;
+ const isValid = this.isValidLimit(textValue);
+ const errorMsg = 'Row limit must be positive integer' +
+ (this.props.maxRow ? ` and not greater than ${this.props.maxRow}` : '');
+ return (
+
+
+
+
+ this.setState({ textValue: e.target.value })}
+ />
+
+
+
+
+
+
+
+ );
+ }
+ render() {
+ return (
+
+
+
+ {this.renderPopover()}
+
+
+ );
+ }
+ }
+ LimitControl.propTypes = propTypes;
diff --git a/superset/assets/src/SqlLab/components/SqlEditor.jsx b/superset/assets/src/SqlLab/components/SqlEditor.jsx
index 37626a83bc280..111c924fb12a1 100644
--- a/superset/assets/src/SqlLab/components/SqlEditor.jsx
+++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx
@@ -16,6 +16,7 @@ import {
import SplitPane from 'react-split-pane';
import Button from '../../components/Button';
+import LimitControl from './LimitControl';
import TemplateParamsEditor from './TemplateParamsEditor';
import SouthPane from './SouthPane';
import SaveQuery from './SaveQuery';
@@ -39,6 +40,8 @@ const propTypes = {
dataPreviewQueries: PropTypes.array.isRequired,
queryEditor: PropTypes.object.isRequired,
hideLeftBar: PropTypes.bool,
+ defaultQueryLimit: PropTypes.number.isRequired,
+ maxRow: PropTypes.number.isRequired,
};
const defaultProps = {
@@ -125,6 +128,9 @@ class SqlEditor extends React.PureComponent {
setQueryEditorSql(sql) {
this.props.actions.queryEditorSetSql(this.props.queryEditor, sql);
}
+ setQueryLimit(queryLimit) {
+ this.props.actions.queryEditorSetQueryLimit(this.props.queryEditor, queryLimit);
+ }
runQuery() {
this.startQuery(!this.props.database.allow_run_sync);
}
@@ -138,6 +144,7 @@ class SqlEditor extends React.PureComponent {
schema: qe.schema,
tempTableName: ctas ? this.state.ctas : '',
templateParams: qe.templateParams,
+ queryLimit: qe.queryLimit,
runAsync,
ctas,
};
@@ -231,7 +238,18 @@ class SqlEditor extends React.PureComponent {
- {ctasControls}
+
+ {ctasControls}
+
+
+
+
}
@@ -239,6 +243,8 @@ function mapStateToProps(state) {
tabHistory: state.tabHistory,
tables: state.tables,
defaultDbId: state.defaultDbId,
+ defaultQueryLimit: common.conf.DEFAULT_SQLLAB_LIMIT,
+ maxRow: common.conf.SQL_MAX_ROW,
};
}
function mapDispatchToProps(dispatch) {
diff --git a/superset/assets/src/SqlLab/reducers.js b/superset/assets/src/SqlLab/reducers.js
index f01f2c3bb73a2..bc2eab2b4adff 100644
--- a/superset/assets/src/SqlLab/reducers.js
+++ b/superset/assets/src/SqlLab/reducers.js
@@ -14,6 +14,7 @@ export function getInitialState(defaultDbId) {
latestQueryId: null,
autorun: false,
dbId: defaultDbId,
+ queryLimit: restBootstrapData.common.conf.DEFAULT_SQLLAB_LIMIT,
};
return {
@@ -46,6 +47,8 @@ export const sqlLabReducer = function (state, action) {
schema: (action.query.schema) ? action.query.schema : null,
autorun: true,
sql: action.query.sql,
+ queryLimit: action.query.queryLimit,
+ maxRow: action.query.maxRow,
};
return sqlLabReducer(state, actions.addQueryEditor(qe));
@@ -211,6 +214,9 @@ export const sqlLabReducer = function (state, action) {
[actions.QUERY_EDITOR_SET_SQL]() {
return alterInArr(state, 'queryEditors', action.queryEditor, { sql: action.sql });
},
+ [actions.QUERY_EDITOR_SET_QUERY_LIMIT]() {
+ return alterInArr(state, 'queryEditors', action.queryEditor, { queryLimit: action.queryLimit });
+ },
[actions.QUERY_EDITOR_SET_TEMPLATE_PARAMS]() {
return alterInArr(state, 'queryEditors', action.queryEditor, { templateParams: action.templateParams });
},
diff --git a/superset/config.py b/superset/config.py
index 63aebe0d1b169..82eacb5b453bb 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -253,6 +253,9 @@
SQL_MAX_ROW = 1000000
DISPLAY_SQL_MAX_ROW = 1000
+# Default row limit for SQL Lab queries
+DEFAULT_SQLLAB_LIMIT = 1000
+
# Maximum number of tables/views displayed in the dropdown window in SQL Lab.
MAX_TABLE_NAMES = 3000
diff --git a/superset/sql_lab.py b/superset/sql_lab.py
index b52e9a95f6b38..793d60e3eb6f8 100644
--- a/superset/sql_lab.py
+++ b/superset/sql_lab.py
@@ -179,7 +179,7 @@ def handle_error(msg):
query.select_as_cta_used = True
elif (query.limit and superset_query.is_select() and
db_engine_spec.limit_method == LimitMethod.WRAP_SQL):
- executed_sql = database.wrap_sql_limit(executed_sql, query.limit)
+ executed_sql = database.apply_limit_to_sql(executed_sql, query.limit)
query.limit_used = True
# Hook to allow environment-specific mutation (usually comments) to the SQL
diff --git a/superset/views/base.py b/superset/views/base.py
index dc2e48f43ae33..bdca9991d88f9 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -27,6 +27,8 @@
FRONTEND_CONF_KEYS = (
'SUPERSET_WEBSERVER_TIMEOUT',
'ENABLE_JAVASCRIPT_CONTROLS',
+ 'DEFAULT_SQLLAB_LIMIT',
+ 'SQL_MAX_ROW',
)
@@ -183,6 +185,42 @@ def _delete(self, pk):
flash(*self.datamodel.message)
self.update_redirect()
+ def delete_permission_views(self, pk):
+ """
+ Delete permission views of database
+ deletes the record with primary_key = pk
+
+ :param pk:
+ record primary key to delete
+ """
+ item = self.datamodel.get(pk, self._base_filters)
+ if not item:
+ return
+
+ view_menu = security_manager.find_view_menu(item.perm)
+ pvs = security_manager.get_session.query(
+ security_manager.permissionview_model).filter_by(
+ view_menu=view_menu).all()
+
+ schema_view_menu = None
+ if hasattr(item, 'schema_perm'):
+ schema_view_menu = security_manager.find_view_menu(item.schema_perm)
+
+ pvs.extend(security_manager.get_session.query(
+ security_manager.permissionview_model).filter_by(
+ view_menu=schema_view_menu).all())
+
+ for pv in pvs:
+ security_manager.get_session.delete(pv)
+
+ if view_menu:
+ security_manager.get_session.delete(view_menu)
+
+ if schema_view_menu:
+ security_manager.get_session.delete(schema_view_menu)
+
+ security_manager.get_session.commit()
+
@action(
'muldelete',
__('Delete'),
diff --git a/superset/views/core.py b/superset/views/core.py
index e3d01f3a51538..3096f0405ddf3 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -293,13 +293,23 @@ class DatabaseView(SupersetModelView, DeleteMixin, YamlExportMixin): # noqa
def pre_add(self, db):
db.set_sqlalchemy_uri(db.sqlalchemy_uri)
+
+ def post_add(self, db):
security_manager.merge_perm('database_access', db.perm)
for schema in db.all_schema_names():
security_manager.merge_perm(
'schema_access', security_manager.get_schema_perm(db, schema))
def pre_update(self, db):
- self.pre_add(db)
+ db.set_sqlalchemy_uri(db.sqlalchemy_uri)
+ if db.perm != db.get_perm():
+ DeleteMixin.delete_permission_views(self, db.id)
+
+ def post_update(self, db):
+ security_manager.merge_perm('database_access', db.perm)
+ for schema in db.all_schema_names():
+ security_manager.merge_perm(
+ 'schema_access', security_manager.get_schema_perm(db, schema))
def _delete(self, pk):
DeleteMixin._delete(self, pk)
@@ -2351,7 +2361,7 @@ def results(self, key):
'{}'.format(rejected_tables)))
payload = utils.zlib_decompress_to_string(blob)
- display_limit = app.config.get('DISPLAY_SQL_MAX_ROW', None)
+ display_limit = app.config.get('DEFAULT_SQLLAB_LIMIT', None)
if display_limit:
payload_json = json.loads(payload)
payload_json['data'] = payload_json['data'][:display_limit]
@@ -2386,6 +2396,12 @@ def sql_json(self):
schema = request.form.get('schema') or None
template_params = json.loads(
request.form.get('templateParams') or '{}')
+ limit = int(request.form.get('queryLimit', 0))
+ if limit < 0:
+ logging.warning(
+ 'Invalid limit of {} specified. Defaulting to max limit.'.format(limit))
+ limit = 0
+ limit = limit or app.config.get('SQL_MAX_ROW')
session = db.session()
mydb = session.query(models.Database).filter_by(id=database_id).first()
@@ -2408,9 +2424,10 @@ def sql_json(self):
tmp_table_name,
)
+ limits = [mydb.db_engine_spec.get_limit_from_sql(sql), limit]
query = Query(
database_id=int(database_id),
- limit=int(app.config.get('SQL_MAX_ROW', None)),
+ limit=min(lim for lim in limits if lim is not None),
sql=sql,
schema=schema,
select_as_cta=request.form.get('select_as_cta') == 'true',
diff --git a/tests/base_tests.py b/tests/base_tests.py
index 3c01f2ef0c5e7..6d0bb8157c14f 100644
--- a/tests/base_tests.py
+++ b/tests/base_tests.py
@@ -210,7 +210,7 @@ def run_sql(self, sql, client_id, user_name=None, raise_on_error=False):
'/superset/sql_json/',
raise_on_error=False,
data=dict(database_id=dbid, sql=sql, select_as_create_as=False,
- client_id=client_id),
+ client_id=client_id, queryLimit=query_limit),
)
if raise_on_error and 'error' in resp:
raise Exception('run_sql failed')
diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py
index 4626f53093559..8329e306eacd1 100644
--- a/tests/sqllab_tests.py
+++ b/tests/sqllab_tests.py
@@ -257,6 +257,29 @@ def test_sqllab_viz(self):
resp = self.get_json_resp('/superset/sqllab_viz/', data=data)
self.assertIn('table_id', resp)
+ def test_sql_limit(self):
+ self.login('admin')
+ test_limit = 1
+ data = self.run_sql(
+ 'SELECT * FROM ab_user',
+ client_id='sql_limit_1')
+ self.assertGreater(len(data['data']), test_limit)
+ data = self.run_sql(
+ 'SELECT * FROM ab_user',
+ client_id='sql_limit_2',
+ query_limit=test_limit)
+ self.assertEquals(len(data['data']), test_limit)
+ data = self.run_sql(
+ 'SELECT * FROM ab_user LIMIT {}'.format(test_limit),
+ client_id='sql_limit_3',
+ query_limit=test_limit + 1)
+ self.assertEquals(len(data['data']), test_limit)
+ data = self.run_sql(
+ 'SELECT * FROM ab_user LIMIT {}'.format(test_limit + 1),
+ client_id='sql_limit_4',
+ query_limit=test_limit)
+ self.assertEquals(len(data['data']), test_limit)
+
if __name__ == '__main__':
unittest.main()