[migration] Stripping leading and trailing whitespace

[john-bodley]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

Similar to #7084 this PR strips leading and trailing whitespace (produced when using the forms) from specific non-NULL string columns. Previously #7084 dealt only with the cases where the values contained only whitespace.

We ran into this issue when trying to debug a security issue were the table names weren't matching due to trailing whitespace. It seems prudent that the records in Superset's schema be trustworthy and it's more desirable to clean this up via a migration than having to call .strip() every time we dealing with records from the database.

Note depending on the state of the database stripping whitespace may lead to duplicate names which could violate uniqueness constraints and cause the migration to fail. Manually intervention may be required. For example the following MySQL query illustrates SQL metrics which are duplicates after spaces (including tabs) are removed:

SELECT 
    table_id, 
    TRIM(TRIM(BOTH '\t' FROM metric_name)) AS metric_name,
    COUNT(1) AS cnt
FROM 
    sql_metrics 
GROUP BY 
    table_id,
    TRIM(TRIM(BOTH '\t' FROM metric_name))
HAVING 
    cnt > 1

TEST PLAN

Ran superset db upgrade (and superset db downgrade) and verified that the leading and trailing whitespaces were removed.

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

to: @etr2460 @graceguo-supercat @michellethomas @mistercrunch

[etr2460]

why can't we just call strip on everything?

[john-bodley]

I'm not exactly sure how SQLAlchemy's ORM works and how it tracks modifications, but I wanted to make sure we were only updating those records which need mutating.

[etr2460]

makes sense, lgtm