Added better display of NULL values in FilterableTable (as in SQL Lab Results)

[semantiDan]

Enhancement (new features, refinement)

Choose one

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

SUMMARY

While working with a large, non-curated and diverse database consisting of many columns and rows per table I found myself painfully trying to distinguish null values from actual strings/numbers.

Whatsmore, by sorting columns that were supposed to be numerical, the null values sometimes took precedence over the order when I was trying to sort said columns.

• If the value of a cell is null, it shows as NULL (italic and light gray) to easily distinguish it from other values in the results table.
• When sorting a column with NULL values, NULL values always come last regardless of the sorting order.

More information can be found in Issue #7983 (Better Visualization and Sorting for NULL values)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before change when sorting by ascending order for totaltracks column:

Before change when sorting by descending order for totaltracks column:

After change when sorting by ascending order for totaltracks column:

After change when sorting by descending order for totaltracks column:

TEST PLAN

Run any query in SQL lab in which the results contain NULL values and sort the column.

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

[etr2460]

Thanks for the PR! a couple comments here

[semantiDan]

Thanks for your review @etr2460 , your recommendations were implemented

[villebro]

Nice @semantiDan ! Just one observation regarding ordering of nulls; traditionally in SQL, nulls are placed first when ascending and last when descending. Showing nulls last on ascending order, a person used to regular SQL might erroneously believe that there aren't any nulls if the nulls are not on screen. Is there some specific reason why nulls shouldn't be shown first on ascending order?

[semantiDan]

Thanks @villebro ,
You are right to assume that many SQL Servers as MS-SQL and MYSQL sort ascending order first by default.
However, other SQL servers like DB2, PostgreSQL, and ORACLE do the opposite.
Link for reference from https://docs.mendix.com

You can still specify in your SQL query the order you wish to see the nulls, i.e.

SELECT column
FROM schema.table
ORDER BY column DESC NULLS LAST
LIMIT 10

or

SELECT column
FROM schema.table
ORDER BY column DESC NULLS FIRST
LIMIT 10

From my personal experience, when interacting with the UI and I'm doing data exploration, I care more about the values than the NULLS. So when I'm looking at a very large table with many columns, although my initial query had a particular order in mind, I still might want to explore the largest or smallest values for other different columns.

I wonder, what would be the exploratory value for seeing NULL values first?

[villebro]

Wow, I had no idea null sorting was this non-standardized! Personally I sometimes instinctively click the sort order twice to check if there are nulls present. Having them systematically last feels slightly biased, as they would never show up on top of the list irrespective of the ordering. Would be interested in hearing other people's thoughts on this.

At any rate the proposed ordering is much better than the current (mis)behaviour, so if nobody else feels strongly about null ordering I'm perfectly fine with this approach.

[codecov-io]

Codecov Report

Merging #8003 into master will decrease coverage by 7.64%.
The diff coverage is 13.33%.

@@            Coverage Diff             @@
##           master    #8003      +/-   ##
==========================================
- Coverage   73.23%   65.58%   -7.65%     
==========================================
  Files         115      469     +354     
  Lines       11839    22437   +10598     
  Branches        0     2438    +2438     
==========================================
+ Hits         8670    14716    +6046     
- Misses       3169     7601    +4432     
- Partials        0      120     +120

Impacted Files	Coverage Δ
...src/components/FilterableTable/FilterableTable.jsx	78.61% <13.33%> (ø)
superset/jinja_context.py	76.69% <0%> (-0.31%)	⬇️
superset/viz.py	71.61% <0%> (-0.16%)	⬇️
superset/assets/src/components/Checkbox.jsx	100% <0%> (ø)
...ations/deckgl/layers/Polygon/PolygonChartPlugin.js	0% <0%> (ø)
...ets/src/dashboard/components/dnd/DragDroppable.jsx	94.59% <0%> (ø)
...c/visualizations/deckgl/layers/Polygon/Polygon.jsx	0% <0%> (ø)
superset/assets/src/components/EditableTitle.jsx	81.53% <0%> (ø)
superset/assets/src/setup/setupPlugins.js	0% <0%> (ø)
...t/assets/src/components/InfoTooltipWithTrigger.jsx	41.66% <0%> (ø)
... and 348 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b380879...b630a5f. Read the comment docs.

[mistercrunch]

👍

[etr2460]

lgtm! thanks for the contribution