feat: view presto row objects in data grid #7436

khtruong · 2019-05-02T21:59:23Z

SUMMARY

A row or array column is displayed in a single column. This was difficult to comprehend if there were many fields in the row or array.

Example:
Name: ColumnA
Data type: row(nested_column1 float, rowA row(rowB row(nested_column2 int)))

Previous Data Grid Display:

This change will separate out row fields into its own column.

New Data Grid Display:

If the array is an array of row objects, we just display the whole thing like before. Ideally we would like to see that information more clearly but that is tracked by another story.

The changes look like a lot but really we are just doing the following 2 things:

Filter out any columns related to arrays of row objects. For example:
Name: ColumnB
Data type: array(row(nested_column int))
We have column objects for ColumnB and ColumnB.nested_column but we ignore the object corresponding to ColumnB.nested_column because we cannot easily create a select statement for such columns.
Create column clauses correctly where column names in dot notation are quoted and we provide a label for the column. For example:
ColumnA.nested_object would be represented as "ColumnA"."nested_object" AS "ColumnA.nested_object"

TEST PLAN

Manually and added unit tests

REVIEWERS

@betodealmeida @DiggidyDave @bearcage @xtinec @datability-io @john-bodley @michellethomas @graceguo-supercat @mistercrunch

@williaster @kristw I made a small change to the data grid where headers are elided from the left if they are too long.

codecov-io · 2019-05-02T22:11:01Z

Codecov Report

Merging #7436 into lyft-release-sp8 will increase coverage by 0.04%.
The diff coverage is 86.11%.

@@                 Coverage Diff                  @@
##           lyft-release-sp8    #7436      +/-   ##
====================================================
+ Coverage                65%   65.04%   +0.04%     
====================================================
  Files                   429      429              
  Lines                 21095    21128      +33     
  Branches               2337     2337              
====================================================
+ Hits                  13712    13743      +31     
- Misses                 7259     7261       +2     
  Partials                124      124

Impacted Files	Coverage Δ
...src/components/FilterableTable/FilterableTable.jsx	`91.01% <ø> (ø)`	⬆️
superset/db_engine_specs.py	`61.45% <86.11%> (+1.22%)`	⬆️

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 51c7f92...b1aface. Read the comment docs.

betodealmeida · 2019-05-03T17:09:32Z

superset/assets/src/components/FilterableTable/FilterableTableStyles.css

@@ -72,3 +72,10 @@
 }
 .even-row { background: #f2f2f2; }
 .odd-row { background: #ffffff; }
+.header-style {
+  direction: rtl;
+  overflow: hidden;


Should we do overflow: scroll here instead? I know it doesn't look nice, but at least it allows people to different between columns that have long names and similar prefixes.

I tried adding that style but it looked extremely bad :(

betodealmeida · 2019-05-03T17:10:08Z

superset/assets/src/components/FilterableTable/FilterableTableStyles.css

@@ -72,3 +72,10 @@
 }
 .even-row { background: #f2f2f2; }
 .odd-row { background: #ffffff; }
+.header-style {
+  direction: rtl;


Do we need this? Superset supports different translations, and potentially some of them could be ltr.

Good point. Maybe I should just make it the standard ellipsis for now to be consistent with the rest of the app because the answer to your question should be implemented throughout the product.

superset/db_engine_specs.py

betodealmeida · 2019-05-03T17:26:41Z

superset/db_engine_specs.py

+    @classmethod
+    def _get_fields(cls, cols: List[dict]) -> List[ColumnClause]:
+        return BaseEngineSpec._get_fields(cols)
+


Why is this needed? Isn't the class derived from BaseEngineSpec?

The class is derived from PrestoEngineSpec, which is then derived from BaseEngineSpec. So we want to call the grandparent's function and not the parent's function.

Ah, got it! :)

betodealmeida · 2019-05-03T17:27:32Z

superset/db_engine_specs.py

+        """
+        We want to filter out columns that correspond to array content because you cannot
+        select array content without an index, which will lead to a large and complicated
+        query. We know which columns to skip because cols is a list provided to us in a


@khtruong, can you explain what you mean with "you cannot select array content without an index, which will lead to a large and complicated query."?

Lol, yeah I'm not surprised that it is still confusing. Let me re-phrase it.

betodealmeida · 2019-05-03T17:28:46Z

@khtruong, this LGTM but I left a few minor comments.

betodealmeida

Thanks, @khtruong

betodealmeida · 2019-05-03T21:57:45Z

superset/db_engine_specs.py

+    @classmethod
+    def _get_fields(cls, cols: List[dict]) -> List[ColumnClause]:
+        return BaseEngineSpec._get_fields(cols)
+


Ah, got it! :)

feat: Scheduling queries from SQL Lab feat: view presto row objects in data grid (apache#7436)

* Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * Incorporate feedback from initial PR (prematurely merged to lyft-release-sp8) (#7415) * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * Workaround for no results returned (#7442) * feat: view presto row objects in data grid (#7436) * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * feat: Scheduling queries from SQL Lab (#7416) * Lightweight pipelines POC * Add docs * Minor fixes * Remove Lyft URL * Use enum * Minor fix * Fix unit tests * Mark props as required

* filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * Added living goods as among the users of Superset (#7407) * Added living goods as among the users of Superset Living Goods is a non profit organisation with operation in africa and the middle east. We work in community health use data heavily on day to day. Superset is our platform of choice for dashboards. * Update README.md * [dashboard] allow user re-order top-level tabs (#7390) * [SQL Lab] Increase timeout threshold for offline check (#7411) * Bump FAB to 2.0.0 (#7323) * Bump FAB to 2.0.0 * [tests] whitelist SecurityApi login and refresh endpoints * [style] Fix, C812 missing trailing commas * [security] Remove SUPERSET_UPDATE_PERMS flag Registering sources needs to be performed after the views are initialized on UPDATE_PERMS=False configuration * [docs] New, FAB_UPDATE_PERMS and flask fab cli * [docs] Fix, db upgrade needs to come first, create-admin needs a db * [cli] New, superset init bootstraps all permissions for FAB and Superset * [style] Fix, flakes * [annotations] Improves UX on annotation validation, start_dttm, end_dttm (#7326) * Setting renderTrigger on label_colors (#7410) * Refactor out controlUtils.js module + unit tests (#7350) * [WiP]refactor out a controlUtils.js file * unit tests * add missing license * Addressing comments * feature: see Presto row and array data types (#7413) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * Removed --console-log and superset runserver (#7421) * Fixes dashboard export button missing download and #7353 (#7427) * Added additional German translations to string file (#6604) * Added additional German translations to string file Updates to German translation files as per directions * Removed messages.json * [fix] Fixing SQL parsing issue (#7374) * add chinese translate (#7402) * Quick fix to address deadlock issue (#7434) * feat: view presto row objects in data grid (#7445) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * Incorporate feedback from initial PR (prematurely merged to lyft-release-sp8) (#7415) * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * feat: Scheduling queries from SQL Lab (#7416) (#7446) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * Incorporate feedback from initial PR (prematurely merged to lyft-release-sp8) (#7415) * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * Workaround for no results returned (#7442) * feat: view presto row objects in data grid (#7436) * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * feat: Scheduling queries from SQL Lab (#7416) * Lightweight pipelines POC * Add docs * Minor fixes * Remove Lyft URL * Use enum * Minor fix * Fix unit tests * Mark props as required * feat: Add `validate_sql_json` endpoint for checking that a given sql query is valid for the chosen database (#7422) (#7462) merge from lyft-release-sp8 to master * Adds missing metric sum__SP_RUR_TOTL (#7452) * Late import for optional lib pyhive (#7471) * Late import for optional lib pyhive * fix * fix: calendar heatmap examples (#7375) Fixing a set of examples that trip on ValueError vs TypeError * bugfix: Improve support for special characters in schema and table names (#7297) * Bugfix to SQL Lab to support tables and schemas with characters that require quoting * Remove debugging prints * Add uri encoding to secondary tables call * Quote schema names for presto * Quote selected_schema on Snowflake, MySQL and Hive * Remove redundant parens * Add python unit tests * Add js unit test * Fix flake8 linting error * [dashboard] After update filter, trigger new queries when charts are visible (#7233) * trigger query when chart is visible * add integration test * fix: alter sql columns to long text #7463 (#7476) Merge lyft-release-sp8@7bfe7bc to master * Refactor ConsoleLog (#7428) * Revised Chinese translation (#7464) * add chinese translate * edit chinese translation * druid connector: avoid using 'dimensions' for scan queries (#7377) After the following PyDruid change (contained in version 0.5.2) the Superset Histogram charts rendered with Druid data are broken: druid-io/pydruid@0a59a70 Bump the pydruid requirements accordingly in setup.py Issue: #7368

feat: view presto row objects in data grid

bbb0b75

pull-request-size bot added the size/L label May 2, 2019

betodealmeida reviewed May 3, 2019

View reviewed changes

superset/db_engine_specs.py Outdated Show resolved Hide resolved

betodealmeida reviewed May 3, 2019

View reviewed changes

superset/db_engine_specs.py Outdated Show resolved Hide resolved

betodealmeida reviewed May 3, 2019

View reviewed changes

khtruong added 2 commits May 3, 2019 13:42

fix: address feedback

ff6c208

fix: spacing

b1aface

betodealmeida approved these changes May 3, 2019

View reviewed changes

betodealmeida merged commit 2a003c1 into apache:lyft-release-sp8 May 3, 2019

DiggidyDave pushed a commit to lyft/incubator-superset that referenced this pull request May 3, 2019

Merge latest from lyft-release-sp8

eec6419

feat: Scheduling queries from SQL Lab feat: view presto row objects in data grid (apache#7436)

DiggidyDave mentioned this pull request May 3, 2019

Merge latest from lyft-release-sp8 lyft/incubator-superset#26

Merged

DiggidyDave pushed a commit to lyft/incubator-superset that referenced this pull request May 3, 2019

Merge latest from lyft-release-sp8

d9a299d

feat: Scheduling queries from SQL Lab feat: view presto row objects in data grid (apache#7436)

DiggidyDave mentioned this pull request May 3, 2019

Merge latest from lyft-release-sp8 lyft/incubator-superset#27

Merged

khtruong mentioned this pull request May 3, 2019

feat: view presto row objects in data grid #7445

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: view presto row objects in data grid #7436

feat: view presto row objects in data grid #7436

khtruong commented May 2, 2019 •

edited

Loading

codecov-io commented May 2, 2019 •

edited

Loading

betodealmeida May 3, 2019

khtruong May 3, 2019

betodealmeida May 3, 2019 •

edited

Loading

khtruong May 3, 2019

betodealmeida May 3, 2019

khtruong May 3, 2019 •

edited

Loading

betodealmeida May 3, 2019

betodealmeida May 3, 2019

khtruong May 3, 2019

betodealmeida commented May 3, 2019

betodealmeida left a comment

betodealmeida May 3, 2019

feat: view presto row objects in data grid #7436

feat: view presto row objects in data grid #7436

Conversation

khtruong commented May 2, 2019 • edited Loading

CATEGORY

SUMMARY

TEST PLAN

REVIEWERS

codecov-io commented May 2, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida May 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khtruong May 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida commented May 3, 2019

betodealmeida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khtruong commented May 2, 2019 •

edited

Loading

codecov-io commented May 2, 2019 •

edited

Loading

betodealmeida May 3, 2019 •

edited

Loading

khtruong May 3, 2019 •

edited

Loading