You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2018/08/09 20:04:13 UTC
[incubator-superset] branch master updated: [sql lab] visualization
flow to detect unaliased columns (#5579)
This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new f4b45f0 [sql lab] visualization flow to detect unaliased columns (#5579)
f4b45f0 is described below
commit f4b45f07c398236f59a00cdc8e91e019627e0773
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Thu Aug 9 13:04:09 2018 -0700
[sql lab] visualization flow to detect unaliased columns (#5579)
* [sql lab] visualization flow to detect unaliased columns
* Addressing comments
---
.../sqllab/ExploreResultsButton_spec.jsx | 22 ++++++++++--
.../assets/spec/javascripts/sqllab/fixtures.js | 39 +++++++++++++++++++++-
.../src/SqlLab/components/ExploreResultsButton.jsx | 38 ++++++++++++++++++++-
3 files changed, 94 insertions(+), 5 deletions(-)
diff --git a/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx b/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
index b57ddd4..1d0448e 100644
--- a/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
@@ -9,7 +9,7 @@ import sinon from 'sinon';
import $ from 'jquery';
import shortid from 'shortid';
-import { queries } from './fixtures';
+import { queries, queryWithBadColumns } from './fixtures';
import { sqlLabReducer } from '../../../src/SqlLab/reducers';
import * as actions from '../../../src/SqlLab/actions';
import ExploreResultsButton from '../../../src/SqlLab/components/ExploreResultsButton';
@@ -60,19 +60,35 @@ describe('ExploreResultsButton', () => {
requiresTime: true,
value: 'bar',
};
- const getExploreResultsButtonWrapper = () => (
- shallow(<ExploreResultsButton {...mockedProps} />, {
+ const getExploreResultsButtonWrapper = (props = mockedProps) => (
+ shallow(<ExploreResultsButton {...props} />, {
context: { store },
}).dive());
it('renders', () => {
expect(React.isValidElement(<ExploreResultsButton />)).to.equal(true);
});
+
it('renders with props', () => {
expect(
React.isValidElement(<ExploreResultsButton {...mockedProps} />),
).to.equal(true);
});
+
+ it('detects bad columns', () => {
+ const wrapper = getExploreResultsButtonWrapper({
+ database,
+ show: true,
+ query: queryWithBadColumns,
+ });
+
+ const badCols = wrapper.instance().getInvalidColumns();
+ expect(badCols).to.deep.equal(['COUNT(*)', '1', '123', 'CASE WHEN 1=1 THEN 1 ELSE 0 END']);
+
+ const msgWrapper = shallow(wrapper.instance().renderInvalidColumnMessage());
+ expect(msgWrapper.find('div')).to.have.length(1);
+ });
+
it('renders a Button', () => {
const wrapper = getExploreResultsButtonWrapper();
expect(wrapper.find(Button)).to.have.length(1);
diff --git a/superset/assets/spec/javascripts/sqllab/fixtures.js b/superset/assets/spec/javascripts/sqllab/fixtures.js
index 510b5d5..f8d8911 100644
--- a/superset/assets/spec/javascripts/sqllab/fixtures.js
+++ b/superset/assets/spec/javascripts/sqllab/fixtures.js
@@ -179,7 +179,7 @@ export const defaultQueryEditor = {
export const queries = [
{
dbId: 1,
- sql: 'SELECT *FROM superset.slices',
+ sql: 'SELECT * FROM superset.slices',
sqlEditorId: 'SJ8YO72R',
tab: 'Demo',
runAsync: false,
@@ -257,6 +257,43 @@ export const queries = [
results: null,
},
];
+export const queryWithBadColumns = {
+ ...queries[0],
+ results: {
+ data: queries[0].results.data,
+ columns: [{
+ is_date: true,
+ is_dim: false,
+ name: 'COUNT(*)',
+ type: 'STRING',
+ }, {
+ is_date: false,
+ is_dim: true,
+ name: 'this_col_is_ok',
+ type: 'STRING',
+ }, {
+ is_date: false,
+ is_dim: true,
+ name: 'a',
+ type: 'STRING',
+ }, {
+ is_date: false,
+ is_dim: true,
+ name: '1',
+ type: 'STRING',
+ }, {
+ is_date: false,
+ is_dim: true,
+ name: '123',
+ type: 'STRING',
+ }, {
+ is_date: false,
+ is_dim: true,
+ name: 'CASE WHEN 1=1 THEN 1 ELSE 0 END',
+ type: 'STRING',
+ }],
+ },
+};
export const databases = {
result: [{
allow_ctas: true,
diff --git a/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx b/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx
index 493b103..f5bbbe8 100644
--- a/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx
+++ b/superset/assets/src/SqlLab/components/ExploreResultsButton.jsx
@@ -33,12 +33,15 @@ class ExploreResultsButton extends React.PureComponent {
};
this.visualize = this.visualize.bind(this);
this.onClick = this.onClick.bind(this);
+ this.getInvalidColumns = this.getInvalidColumns.bind(this);
+ this.renderInvalidColumnMessage = this.renderInvalidColumnMessage.bind(this);
}
onClick() {
const timeout = this.props.timeout;
+ const msg = this.renderInvalidColumnMessage();
if (Math.round(this.getQueryDuration()) > timeout) {
this.dialog.show({
- title: 'Explore',
+ title: t('Explore'),
body: this.renderTimeoutWarning(),
actions: [
Dialog.CancelAction(),
@@ -51,6 +54,19 @@ class ExploreResultsButton extends React.PureComponent {
dialog.hide();
},
});
+ } else if (msg) {
+ this.dialog.show({
+ title: t('Explore'),
+ body: msg,
+ actions: [
+ Dialog.DefaultAction('Ok', () => {}, 'btn-danger'),
+ ],
+ bsSize: 'large',
+ bsStyle: 'warning',
+ onHide: (dialog) => {
+ dialog.hide();
+ },
+ });
} else {
this.visualize();
}
@@ -65,6 +81,10 @@ class ExploreResultsButton extends React.PureComponent {
getQueryDuration() {
return moment.duration(this.props.query.endDttm - this.props.query.startDttm).asSeconds();
}
+ getInvalidColumns() {
+ const re = /^[A-Za-z_]\w*$/;
+ return this.props.query.results.columns.map(col => col.name).filter(col => !re.test(col));
+ }
datasourceName() {
const { query } = this.props;
const uniqueId = shortid.generate();
@@ -126,6 +146,22 @@ class ExploreResultsButton extends React.PureComponent {
{t('feature to store a summarized data set that you can then explore.')}
</Alert>);
}
+ renderInvalidColumnMessage() {
+ const invalidColumns = this.getInvalidColumns();
+ if (invalidColumns.length === 0) {
+ return null;
+ }
+ return (
+ <div>
+ {t('Column name(s) ')}
+ <code><strong>{invalidColumns.join(', ')} </strong></code>
+ {t('cannot be used as a column name. Please use aliases (as in ')}
+ <code>SELECT count(*)
+ <strong>AS my_alias</strong>
+ </code>){' '}
+ {t('limited to alphanumeric characters and underscores')}
+ </div>);
+ }
render() {
return (
<Button