From 7fae3802d719723fa0846dc7f8649884f8769a69 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Tue, 3 Apr 2018 13:50:41 -0400 Subject: [PATCH 1/3] Add separate limit setting for SqlLab Use separate param for wrap sql Get query limit from config unit tests for limit control rendering in sql editor py unit test pg tests Add max rows limit Remove concept of infinity, always require defined limits consistency Assert on validation errors instead of tooltip fix unit tests attempt persist state pr comments and linting --- .../javascripts/sqllab/LimitControl_spec.jsx | 63 +++++++++ .../javascripts/sqllab/SqlEditor_spec.jsx | 21 ++- .../sqllab/TabbedSqlEditors_spec.jsx | 2 + .../spec/javascripts/sqllab/fixtures.js | 4 + .../sqllab/reducers/sqlLab_spec.js | 5 + superset/assets/src/SqlLab/App.jsx | 9 ++ superset/assets/src/SqlLab/actions/sqlLab.js | 7 + .../src/SqlLab/components/LimitControl.jsx | 125 ++++++++++++++++++ .../src/SqlLab/components/SqlEditor.jsx | 20 ++- .../SqlLab/components/TabbedSqlEditors.jsx | 10 +- .../src/SqlLab/reducers/getInitialState.js | 1 + superset/assets/src/SqlLab/reducers/sqlLab.js | 5 + superset/config.py | 4 +- superset/sql_lab.py | 9 +- superset/views/core.py | 14 +- tests/base_tests.py | 5 +- tests/sqllab_tests.py | 23 ++++ 17 files changed, 313 insertions(+), 14 deletions(-) create mode 100644 superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx create mode 100644 superset/assets/src/SqlLab/components/LimitControl.jsx 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..db5082a170db1 --- /dev/null +++ b/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx @@ -0,0 +1,63 @@ +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: 10000, + 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 822192dd2f87d..617b711675688 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -1,7 +1,8 @@ import React from 'react'; import { shallow } from 'enzyme'; -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'; @@ -16,6 +17,10 @@ describe('SqlEditor', () => { getHeight: () => ('100px'), editorQueries: [], dataPreviewQueries: [], + constants: { + defaultQueryLimit: 10000, + maxRow: 100000, + }, }; it('is valid', () => { expect( @@ -26,4 +31,18 @@ describe('SqlEditor', () => { const wrapper = shallow(); expect(wrapper.find(SqlEditorLeftBar)).toHaveLength(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 33d1e476af9c9..b5e74d9749a81 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: 10000, + maxRow: 100000, }; const getWrapper = () => ( shallow(, { diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index 02a6c127f49aa..8a842ceb414a7 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -365,6 +365,10 @@ export const initialState = { workspaceQueries: [], queriesLastUpdate: 0, activeSouthPaneTab: 'Results', + constants: { + defaultQueryLimit: 10000, + maxRow: 100000, + }, }, messageToasts: [], }; diff --git a/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js b/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js index 964daa8fbd927..6791372d41910 100644 --- a/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js +++ b/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js @@ -81,6 +81,11 @@ describe('sqlLabReducer', () => { newState = sqlLabReducer(newState, actions.queryEditorSetSql(qe, sql)); expect(newState.queryEditors[1].sql).toBe(sql); }); + it('should not fail while setting queryLimit', () => { + const queryLimit = 101; + newState = r.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).toBeNull(); diff --git a/superset/assets/src/SqlLab/App.jsx b/superset/assets/src/SqlLab/App.jsx index bd5906ac5b6e2..1f1661452fb34 100644 --- a/superset/assets/src/SqlLab/App.jsx +++ b/superset/assets/src/SqlLab/App.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { createStore, compose, applyMiddleware } from 'redux'; +import persistState from 'redux-localstorage'; import { Provider } from 'react-redux'; import thunkMiddleware from 'redux-thunk'; import { hot } from 'react-hot-loader'; @@ -39,6 +40,14 @@ const sqlLabPersistStateConfig = { }, }; +function loadConstants() { + return (storedState) => { + const newState = Object.assign({}, storedState); + newState.sqlLab.constants = state.constants; + return newState; + }; +} + const store = createStore( rootReducer, initialState, diff --git a/superset/assets/src/SqlLab/actions/sqlLab.js b/superset/assets/src/SqlLab/actions/sqlLab.js index 17e022b8c723c..084aaea80fa4c 100644 --- a/superset/assets/src/SqlLab/actions/sqlLab.js +++ b/superset/assets/src/SqlLab/actions/sqlLab.js @@ -27,6 +27,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'; @@ -141,6 +142,7 @@ export function runQuery(query) { tmp_table_name: query.tempTableName, select_as_cta: query.ctas, templateParams: query.templateParams, + queryLimit: query.queryLimit, }; return SupersetClient.post({ @@ -230,6 +232,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 }; } @@ -325,6 +331,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..ca33e0c4049cb --- /dev/null +++ b/superset/assets/src/SqlLab/components/LimitControl.jsx @@ -0,0 +1,125 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { + Button, + Label, + FormGroup, + FormControl, + Overlay, + Popover, +} from 'react-bootstrap'; +import ControlHeader from '../../explore/components/ControlHeader'; +import { t } from '../../locales'; + +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 e739efa518f71..7a387707c0ba4 100644 --- a/superset/assets/src/SqlLab/components/SqlEditor.jsx +++ b/superset/assets/src/SqlLab/components/SqlEditor.jsx @@ -17,6 +17,7 @@ import SplitPane from 'react-split-pane'; import { t } from '@superset-ui/translation'; import Button from '../../components/Button'; +import LimitControl from './LimitControl'; import TemplateParamsEditor from './TemplateParamsEditor'; import SouthPane from './SouthPane'; import SaveQuery from './SaveQuery'; @@ -38,6 +39,8 @@ const propTypes = { dataPreviewQueries: PropTypes.array.isRequired, queryEditor: PropTypes.object.isRequired, hideLeftBar: PropTypes.bool, + defaultQueryLimit: PropTypes.number.isRequired, + maxRow: PropTypes.number.isRequired, }; const defaultProps = { @@ -130,6 +133,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); } @@ -143,6 +149,7 @@ class SqlEditor extends React.PureComponent { schema: qe.schema, tempTableName: ctas ? this.state.ctas : '', templateParams: qe.templateParams, + queryLimit: qe.queryLimit, runAsync, ctas, }; @@ -236,7 +243,18 @@ class SqlEditor extends React.PureComponent { - {ctasControls} + + {ctasControls} + + + + )} @@ -243,7 +247,7 @@ class TabbedSqlEditors extends React.PureComponent { TabbedSqlEditors.propTypes = propTypes; TabbedSqlEditors.defaultProps = defaultProps; -function mapStateToProps({ sqlLab }) { +function mapStateToProps({ sqlLab, constants }) { return { databases: sqlLab.databases, queryEditors: sqlLab.queryEditors, @@ -252,6 +256,10 @@ function mapStateToProps({ sqlLab }) { tables: sqlLab.tables, defaultDbId: sqlLab.defaultDbId, offline: sqlLab.offline, + defaultQueryLimit: sqlLab.constants && sqlLab.constants.defaultQueryLimit || + constants.defaultQueryLimit, + maxRow: sqlLab.constants && sqlLab.constants.maxRow || + constants.maxRow, }; } function mapDispatchToProps(dispatch) { diff --git a/superset/assets/src/SqlLab/reducers/getInitialState.js b/superset/assets/src/SqlLab/reducers/getInitialState.js index 14bd677d39d8a..ee985ecbf5de8 100644 --- a/superset/assets/src/SqlLab/reducers/getInitialState.js +++ b/superset/assets/src/SqlLab/reducers/getInitialState.js @@ -11,6 +11,7 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) { latestQueryId: null, autorun: false, dbId: defaultDbId, + queryLimit: defaultQueryLimit, }; return { diff --git a/superset/assets/src/SqlLab/reducers/sqlLab.js b/superset/assets/src/SqlLab/reducers/sqlLab.js index b8a27a97d4153..ff881c327a3ee 100644 --- a/superset/assets/src/SqlLab/reducers/sqlLab.js +++ b/superset/assets/src/SqlLab/reducers/sqlLab.js @@ -32,6 +32,8 @@ export default function sqlLabReducer(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)); @@ -204,6 +206,9 @@ export default function sqlLabReducer(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 c94373969f52e..e20724e7a81f1 100644 --- a/superset/config.py +++ b/superset/config.py @@ -282,8 +282,8 @@ # in the results backend. This also becomes the limit when exporting CSVs SQL_MAX_ROW = 100000 -# Limit to be returned to the frontend. -DISPLAY_MAX_ROW = 1000 +# Default row limit for SQL Lab queries +DEFAULT_SQLLAB_LIMIT = 10000 # 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 d211f025c5b73..1d4171b175780 100644 --- a/superset/sql_lab.py +++ b/superset/sql_lab.py @@ -150,10 +150,11 @@ def handle_error(msg): query.user_id, start_dttm.strftime('%Y_%m_%d_%H_%M_%S')) executed_sql = superset_query.as_create_table(query.tmp_table_name) query.select_as_cta_used = True - if (superset_query.is_select() and SQL_MAX_ROWS and - (not query.limit or query.limit > SQL_MAX_ROWS)): - query.limit = SQL_MAX_ROWS - executed_sql = database.apply_limit_to_sql(executed_sql, query.limit) + if superset_query.is_select(): + if SQL_MAX_ROWS and (not query.limit or query.limit > SQL_MAX_ROWS): + query.limit = SQL_MAX_ROWS + if query.limit: + executed_sql = database.apply_limit_to_sql(executed_sql, query.limit) # Hook to allow environment-specific mutation (usually comments) to the SQL SQL_QUERY_MUTATOR = config.get('SQL_QUERY_MUTATOR') diff --git a/superset/views/core.py b/superset/views/core.py index a16bceb4c289a..cd91cf91761fa 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2456,7 +2456,7 @@ def results(self, key): '{}'.format(rejected_tables)), status=403) payload = utils.zlib_decompress_to_string(blob) - display_limit = app.config.get('DISPLAY_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] @@ -2495,6 +2495,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() @@ -2520,10 +2526,10 @@ def sql_json(self): ) client_id = request.form.get('client_id') or utils.shortid()[:10] - + limits = [mydb.db_engine_spec.get_limit_from_sql(sql), limit] query = Query( database_id=int(database_id), - limit=mydb.db_engine_spec.get_limit_from_sql(sql), + 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', @@ -2838,6 +2844,8 @@ def sqllab(self): """SQL Editor""" d = { 'defaultDbId': config.get('SQLLAB_DEFAULT_DBID'), + 'defaultQueryLimit': config.get('DEFAULT_SQLLAB_LIMIT'), + 'maxRow': config.get('SQL_MAX_ROW'), 'common': self.common_bootsrap_payload(), } return self.render_template( diff --git a/tests/base_tests.py b/tests/base_tests.py index b9b4649a50dbe..7ff1036e321d5 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -154,7 +154,8 @@ def revoke_public_access_to_table(self, table): perm.view_menu and table.perm in perm.view_menu.name): security_manager.del_permission_role(public_role, perm) - def run_sql(self, sql, client_id=None, user_name=None, raise_on_error=False): + def run_sql(self, sql, client_id=None, user_name=None, raise_on_error=False, + query_limit=None): if user_name: self.logout() self.login(username=(user_name if user_name else 'admin')) @@ -163,7 +164,7 @@ def run_sql(self, sql, client_id=None, 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 ca2063cf12d3c..95e3fbc144fd0 100644 --- a/tests/sqllab_tests.py +++ b/tests/sqllab_tests.py @@ -260,6 +260,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() From dd66a8e36816310da654e942264ef03238714846 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Wed, 7 Nov 2018 13:37:56 -0500 Subject: [PATCH 2/3] load configs in via common param --- .../assets/spec/javascripts/sqllab/SqlEditor_spec.jsx | 6 ++---- superset/assets/spec/javascripts/sqllab/fixtures.js | 10 ++++++---- .../spec/javascripts/sqllab/reducers/sqlLab_spec.js | 2 +- superset/assets/src/SqlLab/App.jsx | 9 --------- superset/assets/src/SqlLab/components/LimitControl.jsx | 3 ++- .../assets/src/SqlLab/components/TabbedSqlEditors.jsx | 8 +++----- superset/assets/src/SqlLab/reducers/getInitialState.js | 2 +- superset/views/base.py | 2 ++ superset/views/core.py | 2 -- 9 files changed, 17 insertions(+), 27 deletions(-) diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx index 617b711675688..1b00533d1ce85 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -17,10 +17,8 @@ describe('SqlEditor', () => { getHeight: () => ('100px'), editorQueries: [], dataPreviewQueries: [], - constants: { - defaultQueryLimit: 10000, - maxRow: 100000, - }, + defaultQueryLimit: 10000, + maxRow: 100000, }; it('is valid', () => { expect( diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index 8a842ceb414a7..3972afd0995d8 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -365,12 +365,14 @@ export const initialState = { workspaceQueries: [], queriesLastUpdate: 0, activeSouthPaneTab: 'Results', - constants: { - defaultQueryLimit: 10000, - maxRow: 100000, - }, }, messageToasts: [], + common: { + conf: { + DEFAULT_SQLLAB_LIMIT: 10000, + SQL_MAX_ROW: 100000, + }, + }, }; export const query = { diff --git a/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js b/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js index 6791372d41910..ba189ca0488ea 100644 --- a/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js +++ b/superset/assets/spec/javascripts/sqllab/reducers/sqlLab_spec.js @@ -83,7 +83,7 @@ describe('sqlLabReducer', () => { }); it('should not fail while setting queryLimit', () => { const queryLimit = 101; - newState = r.sqlLabReducer(newState, actions.queryEditorSetQueryLimit(qe, queryLimit)); + newState = sqlLabReducer(newState, actions.queryEditorSetQueryLimit(qe, queryLimit)); expect(newState.queryEditors[1].queryLimit).toEqual(queryLimit); }); it('should set selectedText', () => { diff --git a/superset/assets/src/SqlLab/App.jsx b/superset/assets/src/SqlLab/App.jsx index 1f1661452fb34..bd5906ac5b6e2 100644 --- a/superset/assets/src/SqlLab/App.jsx +++ b/superset/assets/src/SqlLab/App.jsx @@ -1,6 +1,5 @@ import React from 'react'; import { createStore, compose, applyMiddleware } from 'redux'; -import persistState from 'redux-localstorage'; import { Provider } from 'react-redux'; import thunkMiddleware from 'redux-thunk'; import { hot } from 'react-hot-loader'; @@ -40,14 +39,6 @@ const sqlLabPersistStateConfig = { }, }; -function loadConstants() { - return (storedState) => { - const newState = Object.assign({}, storedState); - newState.sqlLab.constants = state.constants; - return newState; - }; -} - const store = createStore( rootReducer, initialState, diff --git a/superset/assets/src/SqlLab/components/LimitControl.jsx b/superset/assets/src/SqlLab/components/LimitControl.jsx index ca33e0c4049cb..cdb1f5750de5b 100644 --- a/superset/assets/src/SqlLab/components/LimitControl.jsx +++ b/superset/assets/src/SqlLab/components/LimitControl.jsx @@ -8,8 +8,9 @@ import { Overlay, Popover, } from 'react-bootstrap'; +import { t } from '@superset-ui/translation'; + import ControlHeader from '../../explore/components/ControlHeader'; -import { t } from '../../locales'; const propTypes = { value: PropTypes.number, diff --git a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx index 4c26962c58901..7407dcbcfc35a 100644 --- a/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx +++ b/superset/assets/src/SqlLab/components/TabbedSqlEditors.jsx @@ -247,7 +247,7 @@ class TabbedSqlEditors extends React.PureComponent { TabbedSqlEditors.propTypes = propTypes; TabbedSqlEditors.defaultProps = defaultProps; -function mapStateToProps({ sqlLab, constants }) { +function mapStateToProps({ sqlLab, common }) { return { databases: sqlLab.databases, queryEditors: sqlLab.queryEditors, @@ -256,10 +256,8 @@ function mapStateToProps({ sqlLab, constants }) { tables: sqlLab.tables, defaultDbId: sqlLab.defaultDbId, offline: sqlLab.offline, - defaultQueryLimit: sqlLab.constants && sqlLab.constants.defaultQueryLimit || - constants.defaultQueryLimit, - maxRow: sqlLab.constants && sqlLab.constants.maxRow || - constants.maxRow, + defaultQueryLimit: common.conf.DEFAULT_SQLLAB_LIMIT, + maxRow: common.conf.SQL_MAX_ROW, }; } function mapDispatchToProps(dispatch) { diff --git a/superset/assets/src/SqlLab/reducers/getInitialState.js b/superset/assets/src/SqlLab/reducers/getInitialState.js index ee985ecbf5de8..3351a22a5c03c 100644 --- a/superset/assets/src/SqlLab/reducers/getInitialState.js +++ b/superset/assets/src/SqlLab/reducers/getInitialState.js @@ -11,7 +11,7 @@ export default function getInitialState({ defaultDbId, ...restBootstrapData }) { latestQueryId: null, autorun: false, dbId: defaultDbId, - queryLimit: defaultQueryLimit, + queryLimit: restBootstrapData.common.conf.DEFAULT_SQLLAB_LIMIT, }; return { diff --git a/superset/views/base.py b/superset/views/base.py index 4cba8b23df888..4b4926c959cab 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -24,6 +24,8 @@ 'SUPERSET_WEBSERVER_TIMEOUT', 'SUPERSET_DASHBOARD_POSITION_DATA_LIMIT', 'ENABLE_JAVASCRIPT_CONTROLS', + 'DEFAULT_SQLLAB_LIMIT', + 'SQL_MAX_ROW', ) diff --git a/superset/views/core.py b/superset/views/core.py index cd91cf91761fa..1f11e704b4e7b 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2844,8 +2844,6 @@ def sqllab(self): """SQL Editor""" d = { 'defaultDbId': config.get('SQLLAB_DEFAULT_DBID'), - 'defaultQueryLimit': config.get('DEFAULT_SQLLAB_LIMIT'), - 'maxRow': config.get('SQL_MAX_ROW'), 'common': self.common_bootsrap_payload(), } return self.render_template( From 9b886afd0eca8c1ce953a85a6ece7668ff3a64b2 Mon Sep 17 00:00:00 2001 From: Jeffrey Wang Date: Wed, 7 Nov 2018 18:18:06 -0500 Subject: [PATCH 3/3] default to 1k --- superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx | 2 +- superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx | 2 +- .../assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx | 2 +- superset/assets/spec/javascripts/sqllab/fixtures.js | 2 +- superset/config.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx b/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx index db5082a170db1..7869eae227e6d 100644 --- a/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/LimitControl_spec.jsx @@ -8,7 +8,7 @@ import ControlHeader from '../../../src/explore/components/ControlHeader'; describe('LimitControl', () => { const defaultProps = { value: 0, - defaultQueryLimit: 10000, + defaultQueryLimit: 1000, maxRow: 100000, onChange: () => {}, }; diff --git a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx index 1b00533d1ce85..95db9b05dea06 100644 --- a/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/SqlEditor_spec.jsx @@ -17,7 +17,7 @@ describe('SqlEditor', () => { getHeight: () => ('100px'), editorQueries: [], dataPreviewQueries: [], - defaultQueryLimit: 10000, + defaultQueryLimit: 1000, maxRow: 100000, }; it('is valid', () => { diff --git a/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx b/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx index b5e74d9749a81..111aba9499fed 100644 --- a/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx +++ b/superset/assets/spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx @@ -52,7 +52,7 @@ describe('TabbedSqlEditors', () => { editorHeight: '', getHeight: () => ('100px'), database: {}, - defaultQueryLimit: 10000, + defaultQueryLimit: 1000, maxRow: 100000, }; const getWrapper = () => ( diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js index 3972afd0995d8..99ba6a8e6814d 100644 --- a/superset/assets/spec/javascripts/sqllab/fixtures.js +++ b/superset/assets/spec/javascripts/sqllab/fixtures.js @@ -369,7 +369,7 @@ export const initialState = { messageToasts: [], common: { conf: { - DEFAULT_SQLLAB_LIMIT: 10000, + DEFAULT_SQLLAB_LIMIT: 1000, SQL_MAX_ROW: 100000, }, }, diff --git a/superset/config.py b/superset/config.py index e20724e7a81f1..a34e5d96f436b 100644 --- a/superset/config.py +++ b/superset/config.py @@ -283,7 +283,7 @@ SQL_MAX_ROW = 100000 # Default row limit for SQL Lab queries -DEFAULT_SQLLAB_LIMIT = 10000 +DEFAULT_SQLLAB_LIMIT = 1000 # Maximum number of tables/views displayed in the dropdown window in SQL Lab. MAX_TABLE_NAMES = 3000