[incubator-superset] 34/34: fix(sql-lab): relax column name restrictions (#10816)

[vi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

villebro pushed a commit to branch 0.38
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit f7e2dfa6056acf83b8a32bd1745e03c33d25047b
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Thu Sep 10 07:54:37 2020 +0300

    fix(sql-lab): relax column name restrictions (#10816)
---
 .../sqllab/ExploreResultsButton_spec.jsx           |  8 +-------
 .../spec/javascripts/sqllab/fixtures.ts            | 10 ++++++++++
 .../src/SqlLab/components/ExploreResultsButton.jsx | 23 ++++++++--------------
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx b/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
index 79f8e69..98affc0 100644
--- a/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
+++ b/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx
@@ -94,13 +94,7 @@ describe('ExploreResultsButton', () => {
     });
 
     const badCols = wrapper.instance().getInvalidColumns();
-    expect(badCols).toEqual([
-      'COUNT(*)',
-      '1',
-      '123',
-      'CASE WHEN 1=1 THEN 1 ELSE 0 END',
-      '__TIMESTAMP',
-    ]);
+    expect(badCols).toEqual(['my_dupe_col__2', '__timestamp', '__TIMESTAMP']);
 
     const msgWrapper = shallow(wrapper.instance().renderInvalidColumnMessage());
     expect(msgWrapper.find('div')).toHaveLength(1);
diff --git a/superset-frontend/spec/javascripts/sqllab/fixtures.ts b/superset-frontend/spec/javascripts/sqllab/fixtures.ts
index d1cedf1..d71ffe9 100644
--- a/superset-frontend/spec/javascripts/sqllab/fixtures.ts
+++ b/superset-frontend/spec/javascripts/sqllab/fixtures.ts
@@ -327,6 +327,16 @@ export const queryWithBadColumns = {
         type: 'TIMESTAMP',
       },
       {
+        is_date: false,
+        name: 'my_dupe_col__2',
+        type: 'STRING',
+      },
+      {
+        is_date: true,
+        name: '__timestamp',
+        type: 'TIMESTAMP',
+      },
+      {
         is_date: true,
         name: '__TIMESTAMP',
         type: 'TIMESTAMP',
diff --git a/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx b/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
index 8ecec6f..a9ad353 100644
--- a/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
+++ b/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
@@ -102,13 +102,12 @@ class ExploreResultsButton extends React.PureComponent {
       .asSeconds();
   }
   getInvalidColumns() {
-    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
-    const re3 = /^__timestamp/i; // is not a reserved temporal column alias
+    const re1 = /__\d+$/; // duplicate column name pattern
+    const re2 = /^__timestamp/i; // reserved temporal column alias
 
     return this.props.query.results.selected_columns
       .map(col => col.name)
-      .filter(col => !re1.test(col) || re2.test(col) || re3.test(col));
+      .filter(col => re1.test(col) || re2.test(col));
   }
   datasourceName() {
     const { query } = this.props;
@@ -193,17 +192,11 @@ class ExploreResultsButton extends React.PureComponent {
         <code>
           <strong>{invalidColumns.join(', ')} </strong>
         </code>
-        {t('cannot be used as a column name. Please use aliases (as in ')}
-        <code>
-          SELECT count(*)&nbsp;
-          <strong>AS my_alias</strong>
-        </code>
-        ){' '}
-        {t(`limited to alphanumeric characters and underscores. The alias "__timestamp"
-          used as for the temporal expression and column aliases ending with
-          double underscores followed by a numeric value are not allowed for reasons
-          discussed in Github issue #5739.
-          `)}
+        {t(`cannot be used as a column name. The column name/alias "__timestamp"
+          is reserved for the main temporal expression, and column aliases ending with
+          double underscores followed by a numeric value (e.g. "my_col__1") are reserved
+          for deduplicating duplicate column names. Please use aliases to rename the
+          invalid column names.`)}
       </div>
     );
   }