From 8a9ae811d006c886fa43383cf3c351ac3be3010b Mon Sep 17 00:00:00 2001
From: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Date: Thu, 10 Sep 2020 07:54:37 +0300
Subject: [PATCH] fix(sql-lab): relax column name restrictions (#10816)

---
 .../sqllab/ExploreResultsButton_spec.jsx      |  8 +------
 .../spec/javascripts/sqllab/fixtures.ts       | 10 ++++++++
 .../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 79f8e69ea2..98affc0055 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 d1cedf1aac..d71ffe987d 100644
--- a/superset-frontend/spec/javascripts/sqllab/fixtures.ts
+++ b/superset-frontend/spec/javascripts/sqllab/fixtures.ts
@@ -326,6 +326,16 @@ export const queryWithBadColumns = {
         name: '__TIME',
         type: 'TIMESTAMP',
       },
+      {
+        is_date: false,
+        name: 'my_dupe_col__2',
+        type: 'STRING',
+      },
+      {
+        is_date: true,
+        name: '__timestamp',
+        type: 'TIMESTAMP',
+      },
       {
         is_date: true,
         name: '__TIMESTAMP',
diff --git a/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx b/superset-frontend/src/SqlLab/components/ExploreResultsButton.jsx
index aa35b95e77..1f94eb2697 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>
     );
   }